From 823aecee8d66b9648f0706b2e2e205b0f99ae158 Mon Sep 17 00:00:00 2001 From: SparkyPotato Date: Tue, 31 Jan 2023 22:01:58 +0530 Subject: [PATCH] improve invalid assignment diagnostic --- src/front/wgsl/error.rs | 44 +++++++++++++++++++++++-------------- src/front/wgsl/lower/mod.rs | 39 +++++++++++++++++++++----------- 2 files changed, 54 insertions(+), 29 deletions(-) diff --git a/src/front/wgsl/error.rs b/src/front/wgsl/error.rs index 1c965fc266..a4e6540237 100644 --- a/src/front/wgsl/error.rs +++ b/src/front/wgsl/error.rs @@ -134,7 +134,7 @@ pub enum NumberError { pub enum InvalidAssignmentType { Other, Swizzle, - ImmutableBinding, + ImmutableBinding(Span), } #[derive(Clone, Debug)] @@ -536,21 +536,33 @@ impl<'a> Error<'a> { labels: vec![(span, "expression is not a reference".into())], notes: vec![], }, - Error::InvalidAssignment { span, ty } => ParseError { - message: "invalid left-hand side of assignment".into(), - labels: vec![(span, "cannot assign to this expression".into())], - notes: match ty { - InvalidAssignmentType::Swizzle => vec![ - "WGSL does not support assignments to swizzles".into(), - "consider assigning each component individually".into(), - ], - InvalidAssignmentType::ImmutableBinding => vec![ - format!("'{}' is an immutable binding", &source[span]), - "consider declaring it with `var` instead of `let`".into(), - ], - InvalidAssignmentType::Other => vec![], - }, - }, + Error::InvalidAssignment { span, ty } => { + let (extra_label, notes) = match ty { + InvalidAssignmentType::Swizzle => ( + None, + vec![ + "WGSL does not support assignments to swizzles".into(), + "consider assigning each component individually".into(), + ], + ), + InvalidAssignmentType::ImmutableBinding(binding_span) => ( + Some((binding_span, "this is an immutable binding".into())), + vec![format!( + "consider declaring '{}' with `var` instead of `let`", + &source[binding_span] + )], + ), + InvalidAssignmentType::Other => (None, vec![]), + }; + + ParseError { + message: "invalid left-hand side of assignment".into(), + labels: std::iter::once((span, "cannot assign to this expression".into())) + .chain(extra_label) + .collect(), + notes, + } + } Error::Pointer(what, span) => ParseError { message: format!("{what} must not be a pointer"), labels: vec![(span, "expression is a pointer".into())], diff --git a/src/front/wgsl/lower/mod.rs b/src/front/wgsl/lower/mod.rs index 94b4469a6d..481ea75354 100644 --- a/src/front/wgsl/lower/mod.rs +++ b/src/front/wgsl/lower/mod.rs @@ -5,6 +5,7 @@ use crate::front::wgsl::parse::{ast, conv}; use crate::front::{Emitter, Typifier}; use crate::proc::{ensure_block_returns, Alignment, Layouter, ResolveContext, TypeResolution}; use crate::{Arena, FastHashMap, Handle, Span}; +use indexmap::IndexMap; mod construction; @@ -74,7 +75,9 @@ pub struct StatementContext<'source, 'temp, 'out> { typifier: &'temp mut Typifier, variables: &'out mut Arena, naga_expressions: &'out mut Arena, - named_expressions: &'out mut crate::NamedExpressions, + /// Stores the names of expressions that are assigned in `let` statement + /// Also stores the spans of the names, for use in errors. + named_expressions: &'out mut IndexMap, (String, Span)>, arguments: &'out [crate::FunctionArgument], module: &'out mut crate::Module, } @@ -126,6 +129,19 @@ impl<'a, 'temp> StatementContext<'a, 'temp, '_> { module: self.module, } } + + fn invalid_assignment_type(&self, expr: Handle) -> InvalidAssignmentType { + if let Some(&(_, span)) = self.named_expressions.get(&expr) { + InvalidAssignmentType::ImmutableBinding(span) + } else { + match self.naga_expressions[expr] { + crate::Expression::Swizzle { .. } => InvalidAssignmentType::Swizzle, + crate::Expression::Access { base, .. } => self.invalid_assignment_type(base), + crate::Expression::AccessIndex { base, .. } => self.invalid_assignment_type(base), + _ => InvalidAssignmentType::Other, + } + } + } } /// State for lowering an `ast::Expression` to Naga IR. @@ -737,7 +753,7 @@ impl<'source, 'temp> Lowerer<'source, 'temp> { let mut local_table = FastHashMap::default(); let mut local_variables = Arena::new(); let mut expressions = Arena::new(); - let mut named_expressions = crate::NamedExpressions::default(); + let mut named_expressions = IndexMap::default(); let arguments = f .arguments @@ -748,7 +764,7 @@ impl<'source, 'temp> Lowerer<'source, 'temp> { let expr = expressions .append(crate::Expression::FunctionArgument(i as u32), arg.name.span); local_table.insert(arg.handle, TypedExpression::non_reference(expr)); - named_expressions.insert(expr, arg.name.name.to_string()); + named_expressions.insert(expr, (arg.name.name.to_string(), arg.name.span)); Ok(crate::FunctionArgument { name: Some(arg.name.name.to_string()), @@ -794,7 +810,10 @@ impl<'source, 'temp> Lowerer<'source, 'temp> { result, local_variables, expressions, - named_expressions, + named_expressions: named_expressions + .into_iter() + .map(|(key, (name, _))| (key, name)) + .collect(), body, }; @@ -867,7 +886,8 @@ impl<'source, 'temp> Lowerer<'source, 'temp> { block.extend(emitter.finish(ctx.naga_expressions)); ctx.local_table .insert(l.handle, TypedExpression::non_reference(value)); - ctx.named_expressions.insert(value, l.name.name.to_string()); + ctx.named_expressions + .insert(value, (l.name.name.to_string(), l.name.span)); return Ok(()); } @@ -1068,14 +1088,7 @@ impl<'source, 'temp> Lowerer<'source, 'temp> { let mut value = self.expression(value, ctx.as_expression(block, &mut emitter))?; if !expr.is_reference { - let ty = if ctx.named_expressions.contains_key(&expr.handle) { - InvalidAssignmentType::ImmutableBinding - } else { - match ctx.naga_expressions[expr.handle] { - crate::Expression::Swizzle { .. } => InvalidAssignmentType::Swizzle, - _ => InvalidAssignmentType::Other, - } - }; + let ty = ctx.invalid_assignment_type(expr.handle); return Err(Error::InvalidAssignment { span: ctx.ast_expressions.get_span(target),