Skip to content

Commit

Permalink
Report missing import error (#91)
Browse files Browse the repository at this point in the history
# Objective

If an import is missing from the module it currently just unwraps and
panics without any explanations.

# Solution

Don't unwrap and bubble up the error instead.

The error reporting is a bit weird because it just points at the start
of the file and I'm not sure how to improve it.

---------

Co-authored-by: robtfm <50659922+robtfm@users.noreply.github.com>
  • Loading branch information
IceSentry and robtfm authored Nov 4, 2024
1 parent 5be8be7 commit 7e101db
Show file tree
Hide file tree
Showing 8 changed files with 176 additions and 20 deletions.
88 changes: 68 additions & 20 deletions src/compose/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1318,20 +1318,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)
.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 @@ -1346,17 +1348,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 @@ -1381,6 +1385,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 @@ -1594,12 +1621,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 @@ -1614,6 +1636,18 @@ impl Composer {

let name = name.unwrap_or_default();

let PreprocessOutput { imports, .. } = self
.preprocessor
.preprocess(&sanitized_source, &shader_defs)
.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 @@ -1643,7 +1677,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 @@ -1652,8 +1686,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 Down
68 changes: 68 additions & 0 deletions src/compose/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,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'

0 comments on commit 7e101db

Please sign in to comment.