Skip to content
This repository has been archived by the owner on Jan 29, 2025. It is now read-only.

Commit

Permalink
improve invalid assignment diagnostic
Browse files Browse the repository at this point in the history
  • Loading branch information
SparkyPotato committed Feb 1, 2023
1 parent d3d98c5 commit 823aece
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 29 deletions.
44 changes: 28 additions & 16 deletions src/front/wgsl/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ pub enum NumberError {
pub enum InvalidAssignmentType {
Other,
Swizzle,
ImmutableBinding,
ImmutableBinding(Span),
}

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -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())],
Expand Down
39 changes: 26 additions & 13 deletions src/front/wgsl/lower/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -74,7 +75,9 @@ pub struct StatementContext<'source, 'temp, 'out> {
typifier: &'temp mut Typifier,
variables: &'out mut Arena<crate::LocalVariable>,
naga_expressions: &'out mut Arena<crate::Expression>,
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<Handle<crate::Expression>, (String, Span)>,
arguments: &'out [crate::FunctionArgument],
module: &'out mut crate::Module,
}
Expand Down Expand Up @@ -126,6 +129,19 @@ impl<'a, 'temp> StatementContext<'a, 'temp, '_> {
module: self.module,
}
}

fn invalid_assignment_type(&self, expr: Handle<crate::Expression>) -> 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.
Expand Down Expand Up @@ -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
Expand All @@ -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()),
Expand Down Expand Up @@ -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,
};

Expand Down Expand Up @@ -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(());
}
Expand Down Expand Up @@ -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),
Expand Down

0 comments on commit 823aece

Please sign in to comment.