Skip to content

Commit c2641a7

Browse files
emilkclaude
andcommitted
[wgsl-in] Report remaining type mismatches in WGSL terms
Extend the previous commit to the rest of the places where the WGSL front end deferred a type error to the IR validator, which can only refer to the operands by handle index — and, worse, prints handles from different arenas identically, so that e.g. a `return` mismatch read The `return` expression Some([1]) does not match the declared return type Some([1]) `try_automatic_conversions` used to pass concrete values straight through, on the grounds that reporting them as failed conversions would be misleading. Instead it now reports a plain type mismatch, so every caller — `var`/`let` initializers, `return`, call arguments and composite constructors — gets a diagnostic with spans and WGSL type names, and no future caller can forget to check. Binary operators are checked with `proc::binary_op_accepts_operands`, extracted from the validator so both share one set of typing rules. This also covers the short-circuiting `&&`/`||`, whose operands never appear in a `Binary` expression for the validator to inspect, and compound assignments like `m += v`. Two consequences worth noting: - `return` with no value in a function that declares a return type, and `return` with a value in one that doesn't, are now parse errors. - `1.0 < some_vec` and `1.0 & some_vec` are now rejected at parse time. WGSL has no mixed scalar/vector overloads for the comparison and bitwise operators, and the validator already rejected them, so such shaders never compiled — only the error changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 6194836 commit c2641a7

10 files changed

Lines changed: 699 additions & 267 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ Bottom level categories:
8585

8686
- Fix panics when shader `var<immediate>` size is larger than 256 bytes. By @beicause in [#9725](https://github.com/gfx-rs/wgpu/pull/9725).
8787
- Fix a panic in the SPIR-V frontend when a subgroup collective operation (e.g. `OpGroupNonUniformUMin`) or `OpGroupNonUniformBallot` used an argument whose value needed to be spilled to a temporary variable, such as when the argument was computed inside a loop. By @nazar-pc in [#9957](https://github.com/gfx-rs/wgpu/issues/9957).
88-
- Report WGSL assignments whose value type does not match the assigned-to memory location as a WGSL error naming both types, instead of an IR validation error like `The type of [13] doesn't match the type stored in [10]`. By @emilk in [#9970](https://github.com/gfx-rs/wgpu/pull/9970).
88+
- Report WGSL type mismatches in assignments, `return` statements, function call arguments, composite constructors and binary operators as WGSL errors naming both types, instead of IR validation errors like `The type of [13] doesn't match the type stored in [10]`. By @emilk in [#9970](https://github.com/gfx-rs/wgpu/pull/9970).
8989

9090
#### Validation
9191

naga/src/front/wgsl/error.rs

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,52 @@ pub(crate) enum Error<'a> {
399399
/// first unacceptable argument.
400400
allowed: Vec<String>,
401401
},
402+
/// A value passed to a user-declared function has a type that does not
403+
/// match the corresponding parameter's type.
404+
ParameterTypeMismatch {
405+
/// The name of the function being called.
406+
function: String,
407+
408+
/// The argument expression whose type is wrong.
409+
arg_span: Span,
410+
411+
/// The index of that argument.
412+
arg_index: u32,
413+
414+
/// The declared type of the corresponding parameter.
415+
expected: String,
416+
417+
/// The argument's actual type.
418+
got: String,
419+
},
420+
/// A `return` statement's value does not match the function's declared
421+
/// return type.
422+
ReturnTypeMismatch {
423+
/// The returned expression.
424+
span: Span,
425+
426+
/// The function's declared return type.
427+
expected: String,
428+
429+
/// The returned expression's type.
430+
got: String,
431+
},
432+
/// A `return` statement in a function with no declared return type has a
433+
/// value.
434+
UnexpectedReturnValue {
435+
span: Span,
436+
},
437+
/// A `return` statement in a function with a declared return type has no
438+
/// value.
439+
MissingReturnValue {
440+
span: Span,
441+
442+
/// The function's declared return type.
443+
expected: String,
444+
},
445+
/// A binary operator was applied to operands whose types it does not
446+
/// accept.
447+
InvalidBinaryOperandTypes(Box<InvalidBinaryOperandTypesError>),
402448
FunctionReturnsVoid(Span),
403449
FunctionMustUseUnused(Span),
404450
FunctionMustUseReturnsVoid(Span, Span),
@@ -410,6 +456,7 @@ pub(crate) enum Error<'a> {
410456
ExpectedPositiveArrayLength(Span),
411457
MissingWorkgroupSize(Span),
412458
ConstantEvaluatorError(Box<ConstantEvaluatorError>, Span),
459+
TypeMismatch(Box<TypeMismatchError>),
413460
AutoConversion(Box<AutoConversionError>),
414461
AutoConversionLeafScalar(Box<AutoConversionLeafScalarError>),
415462
ConcretizationFailed(Box<ConcretizationFailedError>),
@@ -505,6 +552,17 @@ impl From<&'static str> for DiagnosticAttributeNotSupportedPosition {
505552
}
506553
}
507554

555+
/// A value's concrete type differs from the type required by its context.
556+
#[derive(Clone, Debug)]
557+
pub(crate) struct TypeMismatchError {
558+
/// Where the required type comes from.
559+
pub dest_span: Span,
560+
pub dest_type: String,
561+
/// The value whose type is wrong.
562+
pub source_span: Span,
563+
pub source_type: String,
564+
}
565+
508566
#[derive(Clone, Debug)]
509567
pub(crate) struct AutoConversionError {
510568
pub dest_span: Span,
@@ -521,6 +579,16 @@ pub(crate) struct AutoConversionLeafScalarError {
521579
pub source_type: String,
522580
}
523581

582+
#[derive(Clone, Debug)]
583+
pub(crate) struct InvalidBinaryOperandTypesError {
584+
/// The operator, as it is spelled in WGSL.
585+
pub op: &'static str,
586+
pub left_span: Span,
587+
pub left_type: String,
588+
pub right_span: Span,
589+
pub right_type: String,
590+
}
591+
524592
#[derive(Clone, Debug)]
525593
pub(crate) struct ConcretizationFailedError {
526594
pub expr_span: Span,
@@ -1114,6 +1182,69 @@ impl<'a> Error<'a> {
11141182

11151183
ParseError { message, labels, notes }
11161184
}
1185+
Error::ParameterTypeMismatch {
1186+
ref function,
1187+
arg_span,
1188+
arg_index,
1189+
ref expected,
1190+
ref got,
1191+
} => ParseError {
1192+
message: format!(
1193+
"wrong type passed as argument #{} to `{function}`",
1194+
arg_index + 1,
1195+
),
1196+
labels: vec![(
1197+
arg_span,
1198+
format!("expected `{expected}`, found `{got}`").into(),
1199+
)],
1200+
notes: vec![],
1201+
},
1202+
Error::ReturnTypeMismatch {
1203+
span,
1204+
ref expected,
1205+
ref got,
1206+
} => ParseError {
1207+
message: format!(
1208+
"the value returned here has type `{got}`, but the function's return type is `{expected}`"
1209+
),
1210+
labels: vec![(span, format!("this expression has type `{got}`").into())],
1211+
notes: vec![],
1212+
},
1213+
Error::UnexpectedReturnValue { span } => ParseError {
1214+
message: "`return` with a value in a function with no return type".to_string(),
1215+
labels: vec![(span, "this function does not return a value".into())],
1216+
notes: vec![],
1217+
},
1218+
Error::MissingReturnValue { span, ref expected } => ParseError {
1219+
message: format!("`return` with no value in a function returning `{expected}`"),
1220+
labels: vec![(span, format!("expected a value of type `{expected}`").into())],
1221+
notes: vec![],
1222+
},
1223+
Error::InvalidBinaryOperandTypes(ref error) => {
1224+
let InvalidBinaryOperandTypesError {
1225+
op,
1226+
left_span,
1227+
ref left_type,
1228+
right_span,
1229+
ref right_type,
1230+
} = **error;
1231+
ParseError {
1232+
message: format!(
1233+
"the `{op}` operator cannot be applied to `{left_type}` and `{right_type}`"
1234+
),
1235+
labels: vec![
1236+
(
1237+
left_span,
1238+
format!("this expression has type `{left_type}`").into(),
1239+
),
1240+
(
1241+
right_span,
1242+
format!("this expression has type `{right_type}`").into(),
1243+
),
1244+
],
1245+
notes: vec![],
1246+
}
1247+
}
11171248
Error::FunctionReturnsVoid(span) => ParseError {
11181249
message: "function does not return any value".to_string(),
11191250
labels: vec![(span, "".into())],
@@ -1191,6 +1322,30 @@ impl<'a> Error<'a> {
11911322
)],
11921323
notes: vec![],
11931324
},
1325+
Error::TypeMismatch(ref error) => {
1326+
// destructuring ensures all fields are handled
1327+
let TypeMismatchError {
1328+
dest_span,
1329+
ref dest_type,
1330+
source_span,
1331+
ref source_type,
1332+
} = **error;
1333+
let mut labels = vec![(
1334+
source_span,
1335+
format!("this expression has type `{source_type}`").into(),
1336+
)];
1337+
if dest_span != source_span {
1338+
labels.push((
1339+
dest_span,
1340+
format!("a value of type `{dest_type}` is required here").into(),
1341+
));
1342+
}
1343+
ParseError {
1344+
message: format!("expected `{dest_type}`, found `{source_type}`"),
1345+
labels,
1346+
notes: vec![],
1347+
}
1348+
}
11941349
Error::AutoConversion(ref error) => {
11951350
// destructuring ensures all fields are handled
11961351
let AutoConversionError {

naga/src/front/wgsl/lower/conversion.rs

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use alloc::{boxed::Box, string::String, vec::Vec};
55
use crate::common::wgsl::{TryToWgsl, TypeContext};
66
use crate::front::wgsl::error::{
77
AutoConversionError, AutoConversionLeafScalarError, ConcretizationFailedError,
8+
TypeMismatchError,
89
};
910
use crate::front::wgsl::Result;
1011
use crate::{Handle, Span};
@@ -14,13 +15,15 @@ impl<'source> super::ExpressionContext<'source, '_, '_> {
1415
///
1516
/// If no conversions are necessary, return `expr` unchanged.
1617
///
17-
/// If automatic conversions cannot convert `expr` to `goal_ty`, return an
18-
/// [`AutoConversion`] error.
18+
/// If `expr`'s type is concrete and differs from `goal_ty`, return a
19+
/// [`TypeMismatch`] error. If it is abstract but automatic conversions
20+
/// cannot convert it to `goal_ty`, return an [`AutoConversion`] error.
1921
///
2022
/// Although the Load Rule is one of the automatic conversions, this
2123
/// function assumes it has already been applied if appropriate, as
2224
/// indicated by the fact that the Rust type of `expr` is not `Typed<_>`.
2325
///
26+
/// [`TypeMismatch`]: super::Error::TypeMismatch
2427
/// [`AutoConversion`]: super::Error::AutoConversion
2528
pub fn try_automatic_conversions(
2629
&mut self,
@@ -36,20 +39,30 @@ impl<'source> super::ExpressionContext<'source, '_, '_> {
3639
let expr_inner = expr_resolution.inner_with(types);
3740
let goal_inner = goal_ty.inner_with(types);
3841

39-
// We can only convert abstract types, so if `expr` is not abstract do not even
40-
// attempt conversion. This allows the validator to catch type errors correctly
41-
// rather than them being misreported as type conversion errors.
42-
// If the type is an array (of an array, etc) then we must check whether the
43-
// type of the innermost array's base type is abstract.
44-
if !expr_inner.is_abstract(types) {
45-
return Ok(expr);
46-
}
47-
4842
// If `expr` already has the requested type, we're done.
4943
if self.module.compare_types(expr_resolution, goal_ty) {
5044
return Ok(expr);
5145
}
5246

47+
// We can only convert abstract types, so if `expr` is not abstract this is
48+
// a plain type mismatch, not a failed conversion. Report it as such, rather
49+
// than misreporting it as a conversion error.
50+
// If the type is an array (of an array, etc) then we must check whether the
51+
// type of the innermost array's base type is abstract.
52+
if !expr_inner.is_abstract(types) {
53+
let source_type = self.type_resolution_to_string(expr_resolution);
54+
let dest_type = self.type_resolution_to_string(goal_ty);
55+
56+
return Err(Box::new(super::Error::TypeMismatch(Box::new(
57+
TypeMismatchError {
58+
dest_span: goal_span,
59+
dest_type,
60+
source_span: expr_span,
61+
source_type,
62+
},
63+
))));
64+
}
65+
5366
let (_expr_scalar, goal_scalar) =
5467
match expr_inner.automatically_converts_to(goal_inner, types) {
5568
Some(scalars) => scalars,

0 commit comments

Comments
 (0)