Skip to content

Commit

Permalink
validates that overrides are resolved
Browse files Browse the repository at this point in the history
  • Loading branch information
kentslaney committed Feb 7, 2025
1 parent 062f5c4 commit 19b9c35
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 30 deletions.
2 changes: 1 addition & 1 deletion naga/src/back/pipeline_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ pub fn process_overrides<'a>(
// recompute their types and other metadata. For the time being,
// do a full re-validation.
let mut validator = Validator::new(ValidationFlags::all(), Capabilities::all());
let module_info = validator.validate(&module)?;
let module_info = validator.validate_resolved_overrides(&module)?;

Ok((Cow::Owned(module), Cow::Owned(module_info)))
}
Expand Down
2 changes: 1 addition & 1 deletion naga/src/valid/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ impl super::Validator {
crate::TypeInner::Scalar { .. } => {}
_ => return Err(ConstExpressionError::InvalidSplatType(value)),
},
_ if global_expr_kind.is_const(handle) || !self.allow_overrides => {
_ if global_expr_kind.is_const(handle) => {
return Err(ConstExpressionError::NonFullyEvaluatedConst)
}
// the constant evaluator will report errors about override-expressions
Expand Down
55 changes: 30 additions & 25 deletions naga/src/valid/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ pub struct Validator {
valid_expression_list: Vec<Handle<crate::Expression>>,
valid_expression_set: HandleSet<crate::Expression>,
override_ids: FastHashSet<u16>,
allow_overrides: bool,
overrides_resolved: bool,

/// A checklist of expressions that must be visited by a specific kind of
/// statement.
Expand Down Expand Up @@ -359,6 +359,12 @@ pub enum ValidationError {
name: String,
source: OverrideError,
},
#[error("Override {handle:?} '{name}' is unresolved")]
UnresolvedOverride {
handle: Handle<crate::Override>,
name: String,
source: ConstExpressionError,
},
#[error("Global variable {handle:?} '{name}' is invalid")]
GlobalVariable {
handle: Handle<crate::GlobalVariable>,
Expand Down Expand Up @@ -465,7 +471,7 @@ impl Validator {
valid_expression_list: Vec::new(),
valid_expression_set: HandleSet::new(),
override_ids: FastHashSet::default(),
allow_overrides: true,
overrides_resolved: false,
needs_visit: HandleSet::new(),
}
}
Expand Down Expand Up @@ -525,10 +531,6 @@ impl Validator {
gctx: crate::proc::GlobalCtx,
mod_info: &ModuleInfo,
) -> Result<(), OverrideError> {
if !self.allow_overrides {
return Err(OverrideError::NotAllowed);
}

let o = &gctx.overrides[handle];

if let Some(id) = o.id {
Expand Down Expand Up @@ -569,18 +571,18 @@ impl Validator {
&mut self,
module: &crate::Module,
) -> Result<ModuleInfo, WithSpan<ValidationError>> {
self.allow_overrides = true;
self.overrides_resolved = false;
self.validate_impl(module)
}

/// Check the given module to be valid.
///
/// With the additional restriction that overrides are not present.
pub fn validate_no_overrides(
/// With the additional restriction that overrides are all resolved.
pub fn validate_resolved_overrides(
&mut self,
module: &crate::Module,
) -> Result<ModuleInfo, WithSpan<ValidationError>> {
self.allow_overrides = false;
self.overrides_resolved = true;
self.validate_impl(module)
}

Expand Down Expand Up @@ -623,20 +625,6 @@ impl Validator {
}
.with_span_handle(handle, &module.types)
})?;
if !self.allow_overrides {
if let crate::TypeInner::Array {
size: crate::ArraySize::Pending(_),
..
} = ty.inner
{
return Err((ValidationError::Type {
handle,
name: ty.name.clone().unwrap_or_default(),
source: TypeError::UnresolvedOverride(handle),
})
.with_span_handle(handle, &module.types));
}
}
mod_info.type_flags.push(ty_info.flags);
self.types[handle.index()] = ty_info;
}
Expand Down Expand Up @@ -691,7 +679,24 @@ impl Validator {
source,
}
.with_span_handle(handle, &module.overrides)
})?
})?;
if self.overrides_resolved {
if let Some(expr) = r#override.init {
self.validate_const_expression(
expr,
module.to_ctx(),
&mod_info,
&global_expr_kind
).map_err(|source| {
ValidationError::UnresolvedOverride {
handle,
name: r#override.name.clone().unwrap_or_default(),
source,
}
.with_span_handle(handle, &module.overrides)
})?;
}
}
}
}

Expand Down
6 changes: 3 additions & 3 deletions naga/tests/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ fn main() {{
);
let module = naga::front::wgsl::parse_str(&source).unwrap();
let err = valid::Validator::new(Default::default(), valid::Capabilities::all())
.validate_no_overrides(&module)
.validate(&module)
.expect_err("module should be invalid");
assert_eq!(err.emit_to_string(&source), expected_err);
}
Expand Down Expand Up @@ -381,7 +381,7 @@ fn incompatible_interpolation_and_sampling_types() {
for (invalid_source, invalid_module, interpolation, sampling, interpolate_attr) in invalid_cases
{
let err = valid::Validator::new(Default::default(), valid::Capabilities::all())
.validate_no_overrides(&invalid_module)
.validate(&invalid_module)
.expect_err(&format!(
"module should be invalid for {interpolate_attr:?}"
));
Expand Down Expand Up @@ -679,7 +679,7 @@ error: Entry point main at Compute is invalid
for (source, expected_err) in cases {
let module = naga::front::wgsl::parse_str(source).unwrap();
let err = valid::Validator::new(Default::default(), valid::Capabilities::all())
.validate_no_overrides(&module)
.validate(&module)
.expect_err("module should be invalid");
println!("{}", err.emit_to_string(source));
assert_eq!(err.emit_to_string(source), expected_err);
Expand Down

0 comments on commit 19b9c35

Please sign in to comment.