From f76c227ceea51536c36aee02dec0ac4d9e52c1ac Mon Sep 17 00:00:00 2001 From: Sarah Date: Fri, 23 Jul 2021 13:15:10 +0000 Subject: [PATCH] parser: fix wgsl expect_const_expr() Bug: tint:1025 Change-Id: I35aab866ffce57681662f59e4b8e701bb03cf718 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/59220 Kokoro: Kokoro Reviewed-by: Ben Clayton Commit-Queue: Sarah Mashayekhi --- src/reader/wgsl/parser_impl.cc | 79 ++++++++++--------- .../wgsl/parser_impl_const_expr_test.cc | 26 +++++- src/reader/wgsl/parser_impl_error_msg_test.cc | 36 ++++++++- .../parser_impl_global_constant_decl_test.cc | 4 +- .../parser_impl_global_variable_decl_test.cc | 2 +- src/reader/wgsl/token.h | 7 +- 6 files changed, 108 insertions(+), 46 deletions(-) diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc index 99e5de9f7d..20c2a97e0e 100644 --- a/src/reader/wgsl/parser_impl.cc +++ b/src/reader/wgsl/parser_impl.cc @@ -2843,48 +2843,51 @@ Maybe ParserImpl::const_literal() { // | const_literal Expect ParserImpl::expect_const_expr() { auto t = peek(); - auto source = t.source(); - - auto type = type_decl(); - if (type.errored) - return Failure::kErrored; - if (type.matched) { - auto params = expect_paren_block( - "type constructor", [&]() -> Expect { - ast::ExpressionList list; - while (continue_parsing()) { - if (peek_is(Token::Type::kParenRight)) { - break; - } - - auto arg = expect_const_expr(); - if (arg.errored) { - return Failure::kErrored; - } - list.emplace_back(arg.value); - - if (!match(Token::Type::kComma)) { - break; - } - } - return list; - }); - - if (params.errored) + if (t.IsLiteral()) { + auto lit = const_literal(); + if (lit.errored) return Failure::kErrored; + if (!lit.matched) + return add_error(peek(), "unable to parse constant literal"); - return create(source, type.value, - params.value); + return create(source, lit.value); + } else if (!t.IsIdentifier() || get_type(t.to_str())) { + if (peek_is(Token::Type::kParenLeft, 1) || + peek_is(Token::Type::kLessThan, 1)) { + auto type = expect_type("const_expr"); + if (type.errored) + return Failure::kErrored; + + auto params = expect_paren_block( + "type constructor", [&]() -> Expect { + ast::ExpressionList list; + while (continue_parsing()) { + if (peek_is(Token::Type::kParenRight)) { + break; + } + + auto arg = expect_const_expr(); + if (arg.errored) { + return Failure::kErrored; + } + list.emplace_back(arg.value); + + if (!match(Token::Type::kComma)) { + break; + } + } + return list; + }); + + if (params.errored) + return Failure::kErrored; + + return create(source, type.value, + params.value); + } } - - auto lit = const_literal(); - if (lit.errored) - return Failure::kErrored; - if (!lit.matched) - return add_error(peek(), "unable to parse constant literal"); - - return create(source, lit.value); + return add_error(peek(), "unable to parse const_expr"); } Maybe ParserImpl::decoration_list() { diff --git a/src/reader/wgsl/parser_impl_const_expr_test.cc b/src/reader/wgsl/parser_impl_const_expr_test.cc index db05cf91f8..3f313abe10 100644 --- a/src/reader/wgsl/parser_impl_const_expr_test.cc +++ b/src/reader/wgsl/parser_impl_const_expr_test.cc @@ -112,7 +112,7 @@ TEST_F(ParserImplTest, ConstExpr_InvalidExpr) { ASSERT_TRUE(p->has_error()); ASSERT_TRUE(e.errored); ASSERT_EQ(e.value, nullptr); - EXPECT_EQ(p->error(), "1:15: unable to parse constant literal"); + EXPECT_EQ(p->error(), "1:15: invalid type for const_expr"); } TEST_F(ParserImplTest, ConstExpr_ConstLiteral) { @@ -134,7 +134,29 @@ TEST_F(ParserImplTest, ConstExpr_ConstLiteral_Invalid) { ASSERT_TRUE(p->has_error()); ASSERT_TRUE(e.errored); ASSERT_EQ(e.value, nullptr); - EXPECT_EQ(p->error(), "1:1: unknown type 'invalid'"); + EXPECT_EQ(p->error(), "1:1: unable to parse const_expr"); +} + +TEST_F(ParserImplTest, ConstExpr_RegisteredType) { + auto p = parser("S(0)"); + + auto* mem = Member("m", ty.i32(), ast::DecorationList{}); + auto* s = Structure(Sym("S"), {mem}); + p->register_type("S", s); + + auto e = p->expect_const_expr(); + ASSERT_FALSE(e.errored); + ASSERT_TRUE(e->Is()); + ASSERT_TRUE(e->Is()); +} + +TEST_F(ParserImplTest, ConstExpr_NotRegisteredType) { + auto p = parser("S(0)"); + auto e = p->expect_const_expr(); + ASSERT_TRUE(p->has_error()); + ASSERT_TRUE(e.errored); + ASSERT_EQ(e.value, nullptr); + EXPECT_EQ(p->error(), "1:1: unable to parse const_expr"); } TEST_F(ParserImplTest, ConstExpr_Recursion) { diff --git a/src/reader/wgsl/parser_impl_error_msg_test.cc b/src/reader/wgsl/parser_impl_error_msg_test.cc index 83013a207f..86ae4ffa01 100644 --- a/src/reader/wgsl/parser_impl_error_msg_test.cc +++ b/src/reader/wgsl/parser_impl_error_msg_test.cc @@ -434,7 +434,7 @@ TEST_F(ParserImplErrorTest, FunctionMissingOpenLine) { var a : f32 = bar[0]; return; })", - "test.wgsl:2:17 error: unknown type 'bar'\n" + "test.wgsl:2:17 error: unable to parse const_expr\n" " var a : f32 = bar[0];\n" " ^^^\n" "\n" @@ -473,11 +473,43 @@ TEST_F(ParserImplErrorTest, GlobalDeclConstMissingRParen) { TEST_F(ParserImplErrorTest, GlobalDeclConstBadConstLiteral) { EXPECT("let i : vec2 = vec2(!);", - "test.wgsl:1:31 error: unable to parse constant literal\n" + "test.wgsl:1:31 error: unable to parse const_expr\n" "let i : vec2 = vec2(!);\n" " ^\n"); } +TEST_F(ParserImplErrorTest, GlobalDeclConstBadConstLiteralSpaceLessThan) { + EXPECT("let i = 1 < 2;", + "test.wgsl:1:11 error: expected \';\' for let declaration\n" + "let i = 1 < 2;\n" + " ^\n"); +} + +TEST_F(ParserImplErrorTest, GlobalDeclConstNotConstExpr) { + EXPECT( + "let a = 1;\n" + "let b = a;", + "test.wgsl:2:9 error: unable to parse const_expr\n" + "let b = a;\n" + " ^\n"); +} + +TEST_F(ParserImplErrorTest, GlobalDeclConstNotConstExprWithParn) { + EXPECT( + "let a = 1;\n" + "let b = a();", + "test.wgsl:2:9 error: unable to parse const_expr\n" + "let b = a();\n" + " ^\n"); +} + +TEST_F(ParserImplErrorTest, GlobalDeclConstConstExprRegisteredType) { + EXPECT("let a = S0(0);", + "test.wgsl:1:9 error: unable to parse const_expr\n" + "let a = S0(0);\n" + " ^^\n"); +} + TEST_F(ParserImplErrorTest, GlobalDeclConstExprMaxDepth) { uint32_t kMaxDepth = 128; diff --git a/src/reader/wgsl/parser_impl_global_constant_decl_test.cc b/src/reader/wgsl/parser_impl_global_constant_decl_test.cc index bf48641265..df4bf98891 100644 --- a/src/reader/wgsl/parser_impl_global_constant_decl_test.cc +++ b/src/reader/wgsl/parser_impl_global_constant_decl_test.cc @@ -98,7 +98,7 @@ TEST_F(ParserImplTest, GlobalConstantDecl_InvalidExpression) { EXPECT_TRUE(e.errored); EXPECT_FALSE(e.matched); EXPECT_EQ(e.value, nullptr); - EXPECT_EQ(p->error(), "1:15: unable to parse constant literal"); + EXPECT_EQ(p->error(), "1:15: invalid type for const_expr"); } TEST_F(ParserImplTest, GlobalConstantDecl_MissingExpression) { @@ -111,7 +111,7 @@ TEST_F(ParserImplTest, GlobalConstantDecl_MissingExpression) { EXPECT_TRUE(e.errored); EXPECT_FALSE(e.matched); EXPECT_EQ(e.value, nullptr); - EXPECT_EQ(p->error(), "1:14: unable to parse constant literal"); + EXPECT_EQ(p->error(), "1:14: unable to parse const_expr"); } TEST_F(ParserImplTest, GlobalConstantDec_Override_WithId) { diff --git a/src/reader/wgsl/parser_impl_global_variable_decl_test.cc b/src/reader/wgsl/parser_impl_global_variable_decl_test.cc index cd89cf4d91..ba6b1157b9 100644 --- a/src/reader/wgsl/parser_impl_global_variable_decl_test.cc +++ b/src/reader/wgsl/parser_impl_global_variable_decl_test.cc @@ -152,7 +152,7 @@ TEST_F(ParserImplTest, GlobalVariableDecl_InvalidConstExpr) { EXPECT_TRUE(e.errored); EXPECT_FALSE(e.matched); EXPECT_EQ(e.value, nullptr); - EXPECT_EQ(p->error(), "1:24: unable to parse constant literal"); + EXPECT_EQ(p->error(), "1:24: invalid type for const_expr"); } TEST_F(ParserImplTest, GlobalVariableDecl_InvalidVariableDecl) { diff --git a/src/reader/wgsl/token.h b/src/reader/wgsl/token.h index 467e6ac457..a8cd2133e4 100644 --- a/src/reader/wgsl/token.h +++ b/src/reader/wgsl/token.h @@ -372,7 +372,12 @@ class Token { bool IsEof() const { return type_ == Type::kEOF; } /// @returns true if the token is an identifier bool IsIdentifier() const { return type_ == Type::kIdentifier; } - + /// @returns true if the token is a literal + bool IsLiteral() const { + return type_ == Type::kSintLiteral || type_ == Type::kFalse || + type_ == Type::kUintLiteral || type_ == Type::kTrue || + type_ == Type::kFloatLiteral; + } /// @returns true if token is a 'matNxM' bool IsMatrix() const { return type_ == Type::kMat2x2 || type_ == Type::kMat2x3 ||