Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report missing import error #91

Merged
merged 7 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 71 additions & 35 deletions src/compose/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1288,20 +1288,22 @@ impl Composer {
&mut self,
module_set: &ComposableModuleDefinition,
shader_defs: &HashMap<String, ShaderDefValue>,
) -> Result<ComposableModule, ComposerError> {
) -> Result<ComposableModule, EnsureImportsError> {
let PreprocessOutput {
preprocessed_source,
imports,
} = self
.preprocessor
.preprocess(&module_set.sanitized_source, shader_defs, self.validate)
.map_err(|inner| ComposerError {
inner,
source: ErrSource::Module {
name: module_set.name.to_owned(),
offset: 0,
defs: shader_defs.clone(),
},
.map_err(|inner| {
EnsureImportsError::from(ComposerError {
inner,
source: ErrSource::Module {
name: module_set.name.to_owned(),
offset: 0,
defs: shader_defs.clone(),
},
})
})?;

self.ensure_imports(imports.iter().map(|import| &import.definition), shader_defs)?;
Expand All @@ -1316,17 +1318,19 @@ impl Composer {
&preprocessed_source,
imports,
)
.map_err(|err| err.into())
}

// build required ComposableModules for a given set of shader_defs
fn ensure_imports<'a>(
&mut self,
imports: impl IntoIterator<Item = &'a ImportDefinition>,
shader_defs: &HashMap<String, ShaderDefValue>,
) -> Result<(), ComposerError> {
) -> Result<(), EnsureImportsError> {
for ImportDefinition { import, .. } in imports.into_iter() {
// we've already ensured imports exist when they were added
let module_set = self.module_sets.get(import).unwrap();
let Some(module_set) = self.module_sets.get(import) else {
return Err(EnsureImportsError::MissingImport(import.to_owned()));
};
if module_set.get_module(shader_defs).is_some() {
continue;
}
Expand All @@ -1351,6 +1355,29 @@ impl Composer {
}
}

pub enum EnsureImportsError {
MissingImport(String),
ComposerError(ComposerError),
}

impl EnsureImportsError {
fn into_composer_error(self, err_source: ErrSource) -> ComposerError {
match self {
EnsureImportsError::MissingImport(import) => ComposerError {
inner: ComposerErrorInner::ImportNotFound(import.to_owned(), 0),
source: err_source,
},
EnsureImportsError::ComposerError(err) => err,
}
}
}

impl From<ComposerError> for EnsureImportsError {
fn from(value: ComposerError) -> Self {
EnsureImportsError::ComposerError(value)
}
}

#[derive(Default)]
pub struct ComposableModuleDescriptor<'a> {
pub source: &'a str,
Expand Down Expand Up @@ -1562,12 +1589,7 @@ impl Composer {

let sanitized_source = self.sanitize_and_set_auto_bindings(source);

let PreprocessorMetaData {
name,
defines,
imports,
..
} = self
let PreprocessorMetaData { name, defines, .. } = self
.preprocessor
.get_preprocessor_metadata(&sanitized_source, true)
.map_err(|inner| ComposerError {
Expand All @@ -1582,6 +1604,21 @@ impl Composer {

let name = name.unwrap_or_default();

let PreprocessOutput {
preprocessed_source,
imports,
} = self
.preprocessor
.preprocess(&sanitized_source, &shader_defs, self.validate)
.map_err(|inner| ComposerError {
inner,
source: ErrSource::Constructing {
path: file_path.to_owned(),
source: sanitized_source.to_owned(),
offset: 0,
},
})?;

// make sure imports have been added
// and gather additional defs specified at module level
for (import_name, offset) in imports
Expand Down Expand Up @@ -1611,7 +1648,7 @@ impl Composer {
inner: ComposerErrorInner::ImportNotFound(import_name.clone(), offset),
source: ErrSource::Constructing {
path: file_path.to_owned(),
source: sanitized_source,
source: sanitized_source.to_owned(),
offset: 0,
},
});
Expand All @@ -1620,8 +1657,22 @@ impl Composer {
self.ensure_imports(
imports.iter().map(|import| &import.definition),
&shader_defs,
)?;
self.ensure_imports(additional_imports, &shader_defs)?;
)
.map_err(|err| {
err.into_composer_error(ErrSource::Constructing {
path: file_path.to_owned(),
source: sanitized_source.to_owned(),
offset: 0,
})
})?;
self.ensure_imports(additional_imports, &shader_defs)
.map_err(|err| {
err.into_composer_error(ErrSource::Constructing {
path: file_path.to_owned(),
source: sanitized_source.to_owned(),
offset: 0,
})
})?;

let definition = ComposableModuleDefinition {
name,
Expand All @@ -1637,21 +1688,6 @@ impl Composer {
modules: Default::default(),
};

let PreprocessOutput {
preprocessed_source,
imports,
} = self
.preprocessor
.preprocess(&sanitized_source, &shader_defs, self.validate)
.map_err(|inner| ComposerError {
inner,
source: ErrSource::Constructing {
path: file_path.to_owned(),
source: sanitized_source,
offset: 0,
},
})?;

let composable = self
.create_composable_module(
&definition,
Expand Down
68 changes: 68 additions & 0 deletions src/compose/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1094,6 +1094,74 @@ mod test {
output_eq!(wgsl, "tests/expected/conditional_import_b.txt");
}

#[test]
fn conditional_missing_import() {
let mut composer = Composer::default();

composer
.add_composable_module(ComposableModuleDescriptor {
source: include_str!("tests/conditional_import_fail/mod_a_b.wgsl"),
file_path: "tests/conditional_import_fail/mod_a_b.wgsl",
..Default::default()
})
.unwrap();

let error = composer
.make_naga_module(NagaModuleDescriptor {
source: include_str!("tests/conditional_import_fail/top.wgsl"),
file_path: "tests/conditional_import_fail/top.wgsl",
..Default::default()
})
.err()
.unwrap();

let text = error.emit_to_string(&composer);

// let mut f = std::fs::File::create("conditional_missing_import.txt").unwrap();
// f.write_all(text.as_bytes()).unwrap();
// drop(f);

output_eq!(text, "tests/expected/conditional_missing_import.txt");
}

#[test]
fn conditional_missing_import_nested() {
let mut composer = Composer::default();

composer
.add_composable_module(ComposableModuleDescriptor {
source: include_str!("tests/conditional_import_fail/mod_a_b.wgsl"),
file_path: "tests/conditional_import_fail/mod_a_b.wgsl",
..Default::default()
})
.unwrap();

composer
.add_composable_module(ComposableModuleDescriptor {
source: include_str!("tests/conditional_import_fail/middle.wgsl"),
file_path: "tests/conditional_import_fail/middle.wgsl",
..Default::default()
})
.unwrap();

let error = composer
.make_naga_module(NagaModuleDescriptor {
source: include_str!("tests/conditional_import_fail/top_nested.wgsl"),
file_path: "tests/conditional_import_fail/top_nested.wgsl",
..Default::default()
})
.err()
.unwrap();

let text = error.emit_to_string(&composer);

// let mut f = std::fs::File::create("conditional_missing_import_nested.txt").unwrap();
// f.write_all(text.as_bytes()).unwrap();
// drop(f);

output_eq!(text, "tests/expected/conditional_missing_import_nested.txt");
}

#[cfg(feature = "test_shader")]
#[test]
fn rusty_imports() {
Expand Down
9 changes: 9 additions & 0 deletions src/compose/tests/conditional_import_fail/middle.wgsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#define_import_path middle

#ifdef USE_A
#import a::b
#endif

fn mid_fn() -> u32 {
return b::C;
}
3 changes: 3 additions & 0 deletions src/compose/tests/conditional_import_fail/mod_a_b.wgsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#define_import_path a::b

const C: u32 = 1u;
7 changes: 7 additions & 0 deletions src/compose/tests/conditional_import_fail/top.wgsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#ifdef USE_A
#import a::b
#endif

fn main() -> u32 {
return b::C;
}
5 changes: 5 additions & 0 deletions src/compose/tests/conditional_import_fail/top_nested.wgsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#import middle

fn main() -> u32 {
return middle::mid_fn();
}
8 changes: 8 additions & 0 deletions src/compose/tests/expected/conditional_missing_import.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: required import 'b' not found
┌─ tests/conditional_import_fail/top.wgsl:6:12
6 │ return b::C;
│ ^
= missing import 'b'

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: required import 'b' not found
┌─ tests/conditional_import_fail/top_nested.wgsl:1:1
1 │ #import middle
│ ^
= missing import 'b'

Loading