[naga wgsl-in] Handle struct types in error messages properly.

Don't try to use `&TypeInner` values to generate error messages, since
`&TypeInner` cannot represent struct types in WGSL - they must be
referred to by name, and only `Type` knows the type's name. Using
`Handle<Type>` or `TypeResolution` works fine, so use that instead.

Add a new `Display` impl for `DiagnosticDisplay` for `TypeResolution`.

Fixes #7495.
This commit is contained in:
Jim Blandy
2025-04-08 17:58:46 -07:00
committed by Erich Gubler
parent 1c4b73c098
commit b837db37ad
3 changed files with 46 additions and 10 deletions

View File

@@ -1,6 +1,6 @@
//! Displaying Naga IR terms in diagnostic output.
use crate::proc::{GlobalCtx, Rule};
use crate::proc::{GlobalCtx, Rule, TypeResolution};
use crate::{Handle, Scalar, Type, TypeInner};
#[cfg(any(feature = "wgsl-in", feature = "wgsl-out"))]
@@ -49,6 +49,17 @@ use core::fmt;
/// indication will provoke a compile-time error.
pub struct DiagnosticDisplay<T>(pub T);
impl fmt::Display for DiagnosticDisplay<(&TypeResolution, GlobalCtx<'_>)> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let (resolution, ctx) = self.0;
match *resolution {
TypeResolution::Handle(handle) => DiagnosticDisplay((handle, ctx)).fmt(f),
TypeResolution::Value(ref inner) => DiagnosticDisplay((inner, ctx)).fmt(f),
}
}
}
impl fmt::Display for DiagnosticDisplay<(Handle<Type>, GlobalCtx<'_>)> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let (handle, ref ctx) = self.0;

View File

@@ -7,6 +7,7 @@ use alloc::{
use core::num::NonZeroU32;
use crate::common::wgsl::{TryToWgsl, TypeContext};
use crate::common::ForDebugWithTypes;
use crate::front::wgsl::error::{Error, ExpectedToken, InvalidAssignmentType};
use crate::front::wgsl::index::Index;
use crate::front::wgsl::parse::number::Number;
@@ -3026,13 +3027,14 @@ impl<'source, 'temp> Lowerer<'source, 'temp> {
);
for (arg_index, &arg) in arguments.iter().enumerate() {
let ty = ctx.typifier()[arg].inner_with(&ctx.module.types);
let arg_type_resolution = &ctx.typifier()[arg];
let arg_inner = arg_type_resolution.inner_with(&ctx.module.types);
log::debug!(
"Supplying argument {arg_index} of type {}",
crate::common::DiagnosticDisplay((ty, ctx.module.to_ctx()))
"Supplying argument {arg_index} of type {:?}",
arg_type_resolution.for_debug(&ctx.module.types)
);
let next_remaining_overloads =
remaining_overloads.arg(arg_index, ty, &ctx.module.types);
remaining_overloads.arg(arg_index, arg_inner, &ctx.module.types);
// If any argument is not a constant expression, then no overloads
// that accept abstract values should be considered.
@@ -3052,11 +3054,11 @@ impl<'source, 'temp> Lowerer<'source, 'temp> {
let function = fun.to_wgsl_for_diagnostics();
let call_span = span;
let arg_span = ctx.get_expression_span(arg);
let arg_ty = ctx.as_diagnostic_display(ty).to_string();
let arg_ty = ctx.as_diagnostic_display(arg_type_resolution).to_string();
// Is this type *ever* permitted for the arg_index'th argument?
// For example, `bool` is never permitted for `max`.
let only_this_argument = overloads.arg(arg_index, ty, &ctx.module.types);
let only_this_argument = overloads.arg(arg_index, arg_inner, &ctx.module.types);
if only_this_argument.is_empty() {
// No overload of `fun` accepts this type as the
// arg_index'th argument. Determine the set of types that
@@ -3105,16 +3107,18 @@ impl<'source, 'temp> Lowerer<'source, 'temp> {
// made this one unacceptable.
let mut remaining_overloads = overloads;
for (prior_index, &prior_expr) in arguments.iter().enumerate() {
let prior_ty = ctx.typifier()[prior_expr].inner_with(&ctx.module.types);
let prior_type_resolution = &ctx.typifier()[prior_expr];
let prior_ty = prior_type_resolution.inner_with(&ctx.module.types);
remaining_overloads =
remaining_overloads.arg(prior_index, prior_ty, &ctx.module.types);
if remaining_overloads
.arg(arg_index, ty, &ctx.module.types)
.arg(arg_index, arg_inner, &ctx.module.types)
.is_empty()
{
// This is the argument that killed our dreams.
let inconsistent_span = ctx.get_expression_span(arguments[prior_index]);
let inconsistent_ty = ctx.as_diagnostic_display(prior_ty).to_string();
let inconsistent_ty =
ctx.as_diagnostic_display(prior_type_resolution).to_string();
if allowed.is_empty() {
// Some overloads did accept `ty` at `arg_index`, but

View File

@@ -3369,3 +3369,24 @@ fn more_inconsistent_type() {
variant("clamp(1f, 1i, 1f)");
variant("clamp(1f, 1f, 1i)");
}
/// Naga should not crash just because the type of a
/// bad argument is a struct.
#[test]
fn struct_names_in_argument_errors() {
#[track_caller]
fn variant(argument: &str) -> Result<naga::Module, naga::front::wgsl::ParseError> {
let input = format!(
r#"
struct A {{ x: i32, }};
fn f() {{ _ = sin({argument}); }}
"#
);
naga::front::wgsl::parse_str(&input)
}
assert!(variant("1.0").is_ok());
assert!(variant("1").is_ok());
assert!(variant("1i").is_err());
assert!(variant("A()").is_err());
}