Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

wgsl-in: Improve assignment diagnostics #2056

Merged
merged 5 commits into from
Sep 15, 2022
Merged

wgsl-in: Improve assignment diagnostics #2056

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e9d1718
improved assignment statement errors
SparkyPotato Sep 14, 2022
cfff1ab
hint immutable binding
SparkyPotato Sep 14, 2022
ffc6761
fix clippy
SparkyPotato Sep 14, 2022
55e23c2
add diagnostic tests
SparkyPotato Sep 15, 2022
7096e18
cleanup formatting
SparkyPotato Sep 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/front/wgsl/lexer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ impl<'a> Lexer<'a> {
if next.0 == expected {
Ok(next.1)
} else {
Err(Error::Unexpected(next, ExpectedToken::Token(expected)))
Err(Error::Unexpected(next.1, ExpectedToken::Token(expected)))
}
}

Expand All @@ -288,7 +288,7 @@ impl<'a> Lexer<'a> {
Ok(())
} else {
Err(Error::Unexpected(
next,
next.1,
ExpectedToken::Token(Token::Paren(expected)),
))
}
Expand All @@ -314,7 +314,7 @@ impl<'a> Lexer<'a> {
Err(Error::ReservedIdentifierPrefix(span))
}
(Token::Word(word), span) => Ok((word, span)),
other => Err(Error::Unexpected(other, ExpectedToken::Identifier)),
other => Err(Error::Unexpected(other.1, ExpectedToken::Identifier)),
}
}

Expand Down
110 changes: 82 additions & 28 deletions src/front/wgsl/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ pub enum ExpectedToken<'a> {
Constant,
/// Expected: constant, parenthesized expression, identifier
PrimaryExpression,
/// Expected: assignment, increment/decrement expression
Assignment,
/// Expected: '}', identifier
FieldName,
/// Expected: attribute for a type
Expand All @@ -95,9 +97,16 @@ pub enum NumberError {
UnimplementedF16,
}

#[derive(Copy, Clone, Debug, PartialEq)]
pub enum InvalidAssignmentType {
Other,
Swizzle,
ImmutableBinding,
}

#[derive(Clone, Debug)]
pub enum Error<'a> {
Unexpected(TokenSpan<'a>, ExpectedToken<'a>),
Unexpected(Span, ExpectedToken<'a>),
UnexpectedComponents(Span),
BadNumber(Span, NumberError),
/// A negative signed integer literal where both signed and unsigned,
Expand Down Expand Up @@ -151,6 +160,10 @@ pub enum Error<'a> {
Pointer(&'static str, Span),
NotPointer(Span),
NotReference(&'static str, Span),
InvalidAssignment {
span: Span,
ty: InvalidAssignmentType,
},
ReservedKeyword(Span),
Redefinition {
previous: Span,
Expand All @@ -162,7 +175,7 @@ pub enum Error<'a> {
impl<'a> Error<'a> {
fn as_parse_error(&self, source: &'a str) -> ParseError {
match *self {
Error::Unexpected((_, ref unexpected_span), expected) => {
Error::Unexpected(ref unexpected_span, expected) => {
let expected_str = match expected {
ExpectedToken::Token(token) => {
match token {
Expand Down Expand Up @@ -195,6 +208,7 @@ impl<'a> Error<'a> {
ExpectedToken::Integer => "unsigned/signed integer literal".to_string(),
ExpectedToken::Constant => "constant".to_string(),
ExpectedToken::PrimaryExpression => "expression".to_string(),
ExpectedToken::Assignment => "assignment or increment/decrement".to_string(),
ExpectedToken::FieldName => "field name or a closing curly bracket to signify the end of the struct".to_string(),
ExpectedToken::TypeAttribute => "type attribute".to_string(),
ExpectedToken::Statement => "statement".to_string(),
Expand Down Expand Up @@ -439,6 +453,20 @@ impl<'a> Error<'a> {
labels: vec![(span.clone(), "expression is not a reference".into())],
notes: vec![],
},
Error::InvalidAssignment{ ref span, ty} => ParseError {
message: "invalid left-hand side of assignment".into(),
labels: vec![(span.clone(), "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.clone()]), "consider declaring it with `var` instead of `let`".into()
],
InvalidAssignmentType::Other => vec![],
},
},
Error::Pointer(what, ref span) => ParseError {
message: format!("{} must not be a pointer", what),
labels: vec![(span.clone(), "expression is a pointer".into())],
Expand Down Expand Up @@ -1409,7 +1437,7 @@ impl Parser {
Token::Number(Ok(Number::U32(num))) if uint => Ok(num as i32),
Token::Number(Ok(Number::I32(num))) if !uint => Ok(num),
Token::Number(Err(e)) => Err(Error::BadNumber(token_span.1, e)),
_ => Err(Error::Unexpected(token_span, ExpectedToken::Integer)),
_ => Err(Error::Unexpected(token_span.1, ExpectedToken::Integer)),
}
}

Expand All @@ -1422,7 +1450,7 @@ impl Parser {
}
(Token::Number(Err(e)), span) => Err(Error::BadNumber(span, e)),
other => Err(Error::Unexpected(
other,
other.1,
ExpectedToken::Number(NumberType::I32),
)),
}
Expand All @@ -1439,7 +1467,7 @@ impl Parser {
(Token::Number(Ok(Number::U32(num))), _) => Ok(num),
(Token::Number(Err(e)), span) => Err(Error::BadNumber(span, e)),
other => Err(Error::Unexpected(
other,
other.1,
ExpectedToken::Number(NumberType::I32),
)),
}
Expand Down Expand Up @@ -2239,7 +2267,7 @@ impl Parser {
components,
}
}
other => return Err(Error::Unexpected(other, ExpectedToken::Constant)),
other => return Err(Error::Unexpected(other.1, ExpectedToken::Constant)),
};

// Only set span if it's a named constant. Otherwise, the enclosing Expression should have
Expand Down Expand Up @@ -2330,7 +2358,7 @@ impl Parser {
}
}
}
other => return Err(Error::Unexpected(other, ExpectedToken::PrimaryExpression)),
other => return Err(Error::Unexpected(other.1, ExpectedToken::PrimaryExpression)),
};
Ok(expr)
}
Expand Down Expand Up @@ -2831,7 +2859,7 @@ impl Parser {
while !lexer.skip(Token::Paren('}')) {
if !ready {
return Err(Error::Unexpected(
lexer.next(),
lexer.next().1,
ExpectedToken::Token(Token::Separator(',')),
));
}
Expand Down Expand Up @@ -2863,7 +2891,7 @@ impl Parser {

let (name, span) = match lexer.next() {
(Token::Word(word), span) => (word, span),
other => return Err(Error::Unexpected(other, ExpectedToken::FieldName)),
other => return Err(Error::Unexpected(other.1, ExpectedToken::FieldName)),
};
if crate::keywords::wgsl::RESERVED.contains(&name) {
return Err(Error::ReservedKeyword(span));
Expand Down Expand Up @@ -3276,7 +3304,7 @@ impl Parser {

if lexer.skip(Token::Attribute) {
let other = lexer.next();
return Err(Error::Unexpected(other, ExpectedToken::TypeAttribute));
return Err(Error::Unexpected(other.1, ExpectedToken::TypeAttribute));
}

let (name, name_span) = lexer.next_ident_with_span()?;
Expand All @@ -3300,23 +3328,42 @@ impl Parser {
fn parse_assignment_statement<'a, 'out>(
&mut self,
lexer: &mut Lexer<'a>,
mut context: ExpressionContext<'a, '_, 'out>,
mut context: StatementContext<'a, '_, 'out>,
block: &mut crate::Block,
emitter: &mut super::Emitter,
) -> Result<(), Error<'a>> {
use crate::BinaryOperator as Bo;

let span_start = lexer.start_byte_offset();
context.emitter.start(context.expressions);
let reference = self.parse_unary_expression(lexer, context.reborrow())?;
emitter.start(context.expressions);
let (reference, lhs_span) = self
.parse_general_expression_for_reference(lexer, context.as_expression(block, emitter))?;
let op = lexer.next();
// The left hand side of an assignment must be a reference.
let lhs_span = span_start..lexer.end_byte_offset();
if !reference.is_reference {
return Err(Error::NotReference(
"the left-hand side of an assignment",
lhs_span,
));
if !matches!(
op.0,
Token::Operation('=')
| Token::AssignmentOperation(_)
| Token::IncrementOperation
| Token::DecrementOperation
) {
return Err(Error::Unexpected(lhs_span, ExpectedToken::Assignment));
} else if !reference.is_reference {
let ty = if context.named_expressions.contains_key(&reference.handle) {
InvalidAssignmentType::ImmutableBinding
} else {
match *context.expressions.get_mut(reference.handle) {
crate::Expression::Swizzle { .. } => InvalidAssignmentType::Swizzle,
_ => InvalidAssignmentType::Other,
}
};

return Err(Error::InvalidAssignment { span: lhs_span, ty });
}

let value = match lexer.next() {
let mut context = context.as_expression(block, emitter);

let value = match op {
(Token::Operation('='), _) => {
self.parse_general_expression(lexer, context.reborrow())?
}
Expand Down Expand Up @@ -3406,7 +3453,7 @@ impl Parser {
op_span.into(),
)
}
other => return Err(Error::Unexpected(other, ExpectedToken::SwitchItem)),
other => return Err(Error::Unexpected(other.1, ExpectedToken::SwitchItem)),
};

let span_end = lexer.end_byte_offset();
Expand Down Expand Up @@ -3843,7 +3890,10 @@ impl Parser {
}
(Token::Paren('}'), _) => break,
other => {
return Err(Error::Unexpected(other, ExpectedToken::SwitchItem))
return Err(Error::Unexpected(
other.1,
ExpectedToken::SwitchItem,
))
}
}
}
Expand Down Expand Up @@ -3954,7 +4004,9 @@ impl Parser {
}
_ => self.parse_assignment_statement(
lexer,
context.as_expression(&mut continuing, &mut emitter),
context.reborrow(),
&mut continuing,
&mut emitter,
)?,
}
lexer.expect(Token::Paren(')'))?;
Expand Down Expand Up @@ -4059,7 +4111,9 @@ impl Parser {
match context.symbol_table.lookup(ident) {
Some(_) => self.parse_assignment_statement(
lexer,
context.as_expression(block, &mut emitter),
context,
block,
&mut emitter,
)?,
None => self.parse_function_statement(
lexer,
Expand All @@ -4078,7 +4132,7 @@ impl Parser {
}
_ => {
let mut emitter = super::Emitter::default();
self.parse_assignment_statement(lexer, context.as_expression(block, &mut emitter))?;
self.parse_assignment_statement(lexer, context, block, &mut emitter)?;
self.pop_rule_span(lexer);
}
}
Expand Down Expand Up @@ -4262,7 +4316,7 @@ impl Parser {
while !lexer.skip(Token::Paren(')')) {
if !ready {
return Err(Error::Unexpected(
lexer.next(),
lexer.next().1,
ExpectedToken::Token(Token::Separator(',')),
));
}
Expand Down Expand Up @@ -4391,7 +4445,7 @@ impl Parser {
(Token::Separator(','), _) if i != 2 => (),
other => {
return Err(Error::Unexpected(
other,
other.1,
ExpectedToken::WorkgroupSizeSeparator,
))
}
Expand Down Expand Up @@ -4577,7 +4631,7 @@ impl Parser {
}
}
(Token::End, _) => return Ok(false),
other => return Err(Error::Unexpected(other, ExpectedToken::GlobalItem)),
other => return Err(Error::Unexpected(other.1, ExpectedToken::GlobalItem)),
}

match binding {
Expand Down
80 changes: 80 additions & 0 deletions tests/wgsl-errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1597,3 +1597,83 @@ fn break_if_bad_condition() {
)
}
}

#[test]
fn swizzle_assignment() {
check(
"
fn f() {
var v = vec2(0);
v.xy = vec2(1);
}
",
r###"error: invalid left-hand side of assignment
┌─ wgsl:4:13
│
4 │ v.xy = vec2(1);
│ ^^^^ cannot assign to this expression
│
= note: WGSL does not support assignments to swizzles
= note: consider assigning each component individually

"###,
);
}

#[test]
fn binary_statement() {
check(
"
fn f() {
3 + 5;
}
",
r###"error: expected assignment or increment/decrement, found '3 + 5'
┌─ wgsl:3:13
│
3 │ 3 + 5;
│ ^^^^^ expected assignment or increment/decrement

"###,
);
}

#[test]
fn assign_to_expr() {
check(
"
fn f() {
3 + 5 = 10;
}
",
r###"error: invalid left-hand side of assignment
┌─ wgsl:3:13
│
3 │ 3 + 5 = 10;
│ ^^^^^ cannot assign to this expression

"###,
);
}

#[test]
fn assign_to_let() {
check(
"
fn f() {
let a = 10;
a = 20;
}
",
r###"error: invalid left-hand side of assignment
┌─ wgsl:4:10
│
4 │ a = 20;
│ ^ cannot assign to this expression
│
= note: 'a' is an immutable binding
= note: consider declaring it with `var` instead of `let`

"###,
);
}