From b785dc1e3961c664cbd8b0b6d2d181af5b15f7b3 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Wed, 23 Nov 2022 21:33:11 +0000 Subject: [PATCH] Update parser to match * and & spec change. The * and & operator grammar was updated in the spec to closer match other languages. This CL updates the Tint WGSL parser to match the current spec. Bug: tint:1756 Change-Id: I81b7c373bbd6a540b9273813c63a29487e2907ce Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/111580 Kokoro: Kokoro Commit-Queue: Dan Sinclair Reviewed-by: Ben Clayton --- src/tint/reader/wgsl/parser_impl.cc | 63 +++++++++---------- .../wgsl/parser_impl_lhs_expression_test.cc | 40 +++++++++--- src/tint/resolver/validation_test.cc | 17 +++++ 3 files changed, 81 insertions(+), 39 deletions(-) diff --git a/src/tint/reader/wgsl/parser_impl.cc b/src/tint/reader/wgsl/parser_impl.cc index 7b09931e1b..49e266f1f1 100644 --- a/src/tint/reader/wgsl/parser_impl.cc +++ b/src/tint/reader/wgsl/parser_impl.cc @@ -3227,47 +3227,46 @@ Maybe ParserImpl::core_lhs_expression() { } // lhs_expression -// : ( STAR | AND )* core_lhs_expression component_or_swizzle_specifier? +// : core_lhs_expression component_or_swizzle_specifier ? +// | AND lhs_expression +// | STAR lhs_expression Maybe ParserImpl::lhs_expression() { - std::vector prefixes; - while (peek_is(Token::Type::kStar) || peek_is(Token::Type::kAnd) || - peek_is(Token::Type::kAndAnd)) { - auto& t = next(); - - // If an '&&' is provided split into '&' and '&' - if (t.Is(Token::Type::kAndAnd)) { - split_token(Token::Type::kAnd, Token::Type::kAnd); - } - - prefixes.push_back(&t); - } - auto core_expr = core_lhs_expression(); if (core_expr.errored) { return Failure::kErrored; - } else if (!core_expr.matched) { - if (prefixes.empty()) { - return Failure::kNoMatch; + } + if (core_expr.matched) { + return component_or_swizzle_specifier(core_expr.value); + } + + auto check_lhs = [&](ast::UnaryOp op) -> Maybe { + auto& t = peek(); + auto expr = lhs_expression(); + if (expr.errored) { + return Failure::kErrored; } - - return add_error(peek(), "missing expression"); - } - - const auto* expr = core_expr.value; - for (auto it = prefixes.rbegin(); it != prefixes.rend(); ++it) { - auto& t = **it; - ast::UnaryOp op = ast::UnaryOp::kAddressOf; - if (t.Is(Token::Type::kStar)) { - op = ast::UnaryOp::kIndirection; + if (!expr.matched) { + return add_error(t, "missing expression"); } - expr = create(t.source(), op, expr); + return create(t.source(), op, expr.value); + }; + + // If an `&&` is encountered, split it into two `&`'s + if (match(Token::Type::kAndAnd)) { + // The first `&` is consumed as part of the `&&`, so this needs to run the check itself. + split_token(Token::Type::kAnd, Token::Type::kAnd); + return check_lhs(ast::UnaryOp::kAddressOf); } - auto e = component_or_swizzle_specifier(expr); - if (e.errored) { - return Failure::kErrored; + if (match(Token::Type::kAnd)) { + return check_lhs(ast::UnaryOp::kAddressOf); } - return e.value; + + if (match(Token::Type::kStar)) { + return check_lhs(ast::UnaryOp::kIndirection); + } + + return Failure::kNoMatch; } // variable_updating_statement diff --git a/src/tint/reader/wgsl/parser_impl_lhs_expression_test.cc b/src/tint/reader/wgsl/parser_impl_lhs_expression_test.cc index 6611f3eee1..4e1e0db372 100644 --- a/src/tint/reader/wgsl/parser_impl_lhs_expression_test.cc +++ b/src/tint/reader/wgsl/parser_impl_lhs_expression_test.cc @@ -100,6 +100,31 @@ TEST_F(ParserImplTest, LHSExpression_Multiple) { EXPECT_TRUE(expr->Is()); } +TEST_F(ParserImplTest, LHSExpression_PostfixExpression_Array) { + auto p = parser("*a[0]"); + auto e = p->lhs_expression(); + ASSERT_FALSE(p->has_error()) << p->error(); + ASSERT_FALSE(e.errored); + EXPECT_TRUE(e.matched); + ASSERT_NE(e.value, nullptr); + ASSERT_TRUE(e->Is()); + + auto* u = e->As(); + EXPECT_EQ(u->op, ast::UnaryOp::kIndirection); + + ASSERT_TRUE(u->expr->Is()); + + auto* access = u->expr->As(); + ASSERT_TRUE(access->object->Is()); + + auto* obj = access->object->As(); + EXPECT_EQ(obj->symbol, p->builder().Symbols().Get("a")); + + ASSERT_TRUE(access->index->Is()); + auto* idx = access->index->As(); + EXPECT_EQ(0, idx->value); +} + TEST_F(ParserImplTest, LHSExpression_PostfixExpression) { auto p = parser("*a.foo"); auto e = p->lhs_expression(); @@ -107,16 +132,17 @@ TEST_F(ParserImplTest, LHSExpression_PostfixExpression) { ASSERT_FALSE(e.errored); EXPECT_TRUE(e.matched); ASSERT_NE(e.value, nullptr); - ASSERT_TRUE(e->Is()); + ASSERT_TRUE(e->Is()); - auto* access = e->As(); - ASSERT_TRUE(access->structure->Is()); - - auto* u = access->structure->As(); + auto* u = e->As(); EXPECT_EQ(u->op, ast::UnaryOp::kIndirection); - ASSERT_TRUE(u->expr->Is()); - auto* struct_ident = u->expr->As(); + ASSERT_TRUE(u->expr->Is()); + + auto* access = u->expr->As(); + ASSERT_TRUE(access->structure->Is()); + + auto* struct_ident = access->structure->As(); EXPECT_EQ(struct_ident->symbol, p->builder().Symbols().Get("a")); ASSERT_TRUE(access->member->Is()); diff --git a/src/tint/resolver/validation_test.cc b/src/tint/resolver/validation_test.cc index 2e41bf1ae7..946bcb9cae 100644 --- a/src/tint/resolver/validation_test.cc +++ b/src/tint/resolver/validation_test.cc @@ -1298,6 +1298,23 @@ TEST_F(ResolverTest, U32_Overflow) { EXPECT_EQ(r()->error(), "12:24 error: value 4294967296 cannot be represented as 'u32'"); } +// var a: array; +// *&a[0] = 1; +TEST_F(ResolverTest, PointerIndexing_Fail) { + // var a: array; + // let p = &a; + // *p[0] = 0; + + auto* a = Var("a", ty.array()); + auto* p = AddressOf("a"); + auto* idx = Assign(Deref(IndexAccessor(p, 0_u)), 0_u); + + WrapInFunction(a, idx); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), "error: cannot index type 'ptr, read_write>'"); +} + } // namespace } // namespace tint::resolver