From a0344cc93267d25e93aa5b506f779e5ec2546985 Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Fri, 13 Dec 2024 16:06:17 -0800 Subject: [PATCH] [naga valid]: Clean up validation of `Statement::ImageStore`. Ensure that the type we obtain for the `image` operand is correct: "see through" a binding array to its element type only when `image` is actually an `Access` or `AccessIndex` expression. (This changes the set of programs the validator will pass, but it turns out not to affect the set of WGSL programs that Naga will accept, since the WGSL front end is already checking the types of the `texture` arguments to `textureStore` function calls, in order to decide which overload applies.) Rename the variables to better reflect their values. --- naga/src/valid/function.rs | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/naga/src/valid/function.rs b/naga/src/valid/function.rs index a41ad00299..40160ce6e8 100644 --- a/naga/src/valid/function.rs +++ b/naga/src/valid/function.rs @@ -1012,9 +1012,12 @@ impl super::Validator { } => { //Note: this code uses a lot of `FunctionError::InvalidImageStore`, // and could probably be refactored. - let var = match *context.get_expression(image) { + let global_var; + let image_ty; + match *context.get_expression(image) { crate::Expression::GlobalVariable(var_handle) => { - &context.global_vars[var_handle] + global_var = &context.global_vars[var_handle]; + image_ty = global_var.ty; } // The `image` operand is indexing into a binding array, // so punch through the `Access`* expression and look at @@ -1029,7 +1032,18 @@ impl super::Validator { ) .with_span_handle(image, context.expressions)); }; - &context.global_vars[var_handle] + global_var = &context.global_vars[var_handle]; + + // The global variable must be a binding array. + let Ti::BindingArray { base, .. } = context.types[global_var.ty].inner + else { + return Err(FunctionError::InvalidImageStore( + ExpressionError::ExpectedBindingArrayType(global_var.ty), + ) + .with_span_handle(global_var.ty, context.types)); + }; + + image_ty = base; } _ => { return Err(FunctionError::InvalidImageStore( @@ -1039,24 +1053,18 @@ impl super::Validator { } }; - // Punch through a binding array to get the underlying type. - let global_ty = match context.types[var.ty].inner { - Ti::BindingArray { base, .. } => &context.types[base].inner, - ref inner => inner, - }; - // The `image` operand must be an `Image`. let Ti::Image { class, arrayed, dim, - } = *global_ty + } = context.types[image_ty].inner else { return Err(FunctionError::InvalidImageStore( - ExpressionError::ExpectedImageType(var.ty), + ExpressionError::ExpectedImageType(global_var.ty), ) .with_span() - .with_handle(var.ty, context.types) + .with_handle(global_var.ty, context.types) .with_handle(image, context.expressions)); };