[tint][reader][wgsl]: Improve diagnostics for template error
Detect when expression lists parse as a template list and then fail to parse.
Give a clear diagnostic highlighting what the parser saw, and how to fix.
Fixed: tint:1945
Change-Id: I08be99cf4fc9cbf7e6bd95c2704f4fd385eb2b4d
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/135642
Commit-Queue: Dan Sinclair <dsinclair@chromium.org>
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
Auto-Submit: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
diff --git a/src/tint/reader/wgsl/parser_impl.cc b/src/tint/reader/wgsl/parser_impl.cc
index b7ad4c5..835c305 100644
--- a/src/tint/reader/wgsl/parser_impl.cc
+++ b/src/tint/reader/wgsl/parser_impl.cc
@@ -579,7 +579,7 @@
errored = true;
}
if (decl.matched) {
- if (!expect_attributes_consumed(attrs.value)) {
+ if (expect_attributes_consumed(attrs.value).errored) {
return Failure::kErrored;
}
return kSuccess;
@@ -590,7 +590,7 @@
errored = true;
}
if (str.matched) {
- if (!expect_attributes_consumed(attrs.value)) {
+ if (expect_attributes_consumed(attrs.value).errored) {
return Failure::kErrored;
}
return kSuccess;
@@ -2496,7 +2496,22 @@
if (peek_is(terminator)) {
break;
}
- if (!expect(use, Token::Type::kComma)) {
+
+ // Check if the next token is a template start, which was likely intended as a less-than.
+ if (expect_next_not_template_list(expr->source).errored) {
+ return Failure::kErrored; // expect_next_not_template_list() raised an error.
+ }
+ if (!match(Token::Type::kComma)) {
+ // Next expression is not a terminator or comma, so this is a parse error.
+
+ // Check if last parsed expression was a templated identifier, which was likely indented
+ // as a less-than / greater-than.
+ if (expect_not_templated_ident_expr(expr.value).errored) {
+ return Failure::kErrored; // expect_not_templated_ident_expr() raised an error.
+ }
+
+ // Emit the expected ',' error
+ expect(use, Token::Type::kComma);
return Failure::kErrored;
}
if (peek_is(terminator)) {
@@ -3074,12 +3089,58 @@
}
}
-bool ParserImpl::expect_attributes_consumed(utils::VectorRef<const ast::Attribute*> in) {
+Expect<Void> ParserImpl::expect_attributes_consumed(utils::VectorRef<const ast::Attribute*> in) {
if (in.IsEmpty()) {
- return true;
+ return kSuccess;
}
add_error(in[0]->source, "unexpected attributes");
- return false;
+ return Failure::kErrored;
+}
+
+Expect<Void> ParserImpl::expect_next_not_template_list(const Source& lhs_source) {
+ Source end;
+ if (!match(Token::Type::kTemplateArgsLeft, &end)) {
+ return kSuccess;
+ }
+
+ // Try to find end of template
+ for (size_t i = 0; i < 32; i++) {
+ if (auto& t = peek(i); t.type() == Token::Type::kTemplateArgsRight) {
+ end = t.source();
+ }
+ }
+ Source template_source = lhs_source;
+ template_source.range.end = end.range.end;
+ add_error(template_source, "parsed as template list");
+
+ if (auto rhs = expression(); rhs.matched) {
+ Source lt_source = lhs_source;
+ lt_source.range.end = rhs->source.range.end;
+ add_note(lt_source,
+ "if this is intended to be a less-than expression then wrap in parentheses");
+ }
+ return Failure::kErrored;
+}
+
+Expect<Void> ParserImpl::expect_not_templated_ident_expr(const ast::Expression* expr) {
+ auto* ident_expr = expr->As<ast::IdentifierExpression>();
+ if (!ident_expr) {
+ return kSuccess;
+ }
+ auto* ident = ident_expr->identifier->As<ast::TemplatedIdentifier>();
+ if (!ident) {
+ return kSuccess;
+ }
+
+ add_error(ident->source, "parsed as template list");
+
+ if (auto rhs = expression(); rhs.matched) {
+ Source gt_source = ident->arguments.Back()->source;
+ gt_source.range.end = rhs->source.range.end;
+ add_note(gt_source,
+ "if this is intended to be a greater-than expression then wrap in parentheses");
+ }
+ return Failure::kErrored;
}
// severity_control_name
diff --git a/src/tint/reader/wgsl/parser_impl.h b/src/tint/reader/wgsl/parser_impl.h
index 6d8c408..b167f56 100644
--- a/src/tint/reader/wgsl/parser_impl.h
+++ b/src/tint/reader/wgsl/parser_impl.h
@@ -824,7 +824,28 @@
/// Reports an error if the attribute list `list` is not empty.
/// Used to ensure that all attributes are consumed.
- bool expect_attributes_consumed(utils::VectorRef<const ast::Attribute*> list);
+ Expect<Void> expect_attributes_consumed(utils::VectorRef<const ast::Attribute*> list);
+
+ /// Raises an error if the next token is the start of a template list.
+ /// Used to hint to the user that the parser interpreted the following as a templated identifier
+ /// expression:
+ ///
+ /// ```
+ /// a < b, c >
+ /// ^~~~~~~~
+ /// ```
+ Expect<Void> expect_next_not_template_list(const Source& lhs_source);
+
+ /// Raises an error if the parsed expression is a templated identifier expression
+ /// Used to hint to the user that the parser intepreted the following as a templated identifier
+ /// expression:
+ ///
+ /// ```
+ /// a < b, c > d
+ /// ^^^^^^^^^^
+ /// expr
+ /// ```
+ Expect<Void> expect_not_templated_ident_expr(const ast::Expression* expr);
/// Parses the given enum, providing sensible error messages if the next token does not match
/// any of the enum values.
diff --git a/src/tint/reader/wgsl/parser_impl_error_msg_test.cc b/src/tint/reader/wgsl/parser_impl_error_msg_test.cc
index e5ce8b9..9156931 100644
--- a/src/tint/reader/wgsl/parser_impl_error_msg_test.cc
+++ b/src/tint/reader/wgsl/parser_impl_error_msg_test.cc
@@ -164,6 +164,30 @@
)");
}
+TEST_F(ParserImplErrorTest, CallStmtArgsTreatedAsTemplateLHS) {
+ EXPECT("fn f() { f( a, b.x < c, d > e ); }",
+ R"(test.wgsl:1:16 error: parsed as template list
+fn f() { f( a, b.x < c, d > e ); }
+ ^^^^^^^^^^^^
+
+test.wgsl:1:16 note: if this is intended to be a less-than expression then wrap in parentheses
+fn f() { f( a, b.x < c, d > e ); }
+ ^^^^^^^
+)");
+}
+
+TEST_F(ParserImplErrorTest, CallStmtArgsTreatedAsTemplateRHS) {
+ EXPECT("fn f() { f( a, b < c, d > e ); }",
+ R"(test.wgsl:1:16 error: parsed as template list
+fn f() { f( a, b < c, d > e ); }
+ ^^^^^^^^^^
+
+test.wgsl:1:23 note: if this is intended to be a greater-than expression then wrap in parentheses
+fn f() { f( a, b < c, d > e ); }
+ ^^^^^
+)");
+}
+
TEST_F(ParserImplErrorTest, CallStmtMissingRParen) {
EXPECT("fn f() { f(1.; }",
R"(test.wgsl:1:14 error: expected ',' for function call
