From ee25586aec81e62f22a594fc3ab2669ecfd92c2b Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Mon, 22 Aug 2022 14:06:44 +0000 Subject: [PATCH] Sync `expression` grammar element with WGSL spec. This CL updates the `expression` grammar element with the WGSL spec. Bug: tint:1633 Change-Id: Iad1f16f5d73cf4d9ba53ef638aaad73418712403 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/99822 Reviewed-by: Ben Clayton Commit-Queue: Dan Sinclair Kokoro: Kokoro --- src/tint/BUILD.gn | 1 + src/tint/CMakeLists.txt | 3 +- src/tint/reader/wgsl/parser_impl.cc | 68 +++++ src/tint/reader/wgsl/parser_impl.h | 3 + .../wgsl/parser_impl_expression_test.cc | 254 ++++++++++++++++++ 5 files changed, 328 insertions(+), 1 deletion(-) create mode 100644 src/tint/reader/wgsl/parser_impl_expression_test.cc diff --git a/src/tint/BUILD.gn b/src/tint/BUILD.gn index 9b2fd353cb..22712a3bd5 100644 --- a/src/tint/BUILD.gn +++ b/src/tint/BUILD.gn @@ -1370,6 +1370,7 @@ if (tint_build_unittests) { "reader/wgsl/parser_impl_error_msg_test.cc", "reader/wgsl/parser_impl_error_resync_test.cc", "reader/wgsl/parser_impl_exclusive_or_expression_test.cc", + "reader/wgsl/parser_impl_expression_test.cc", "reader/wgsl/parser_impl_external_texture_test.cc", "reader/wgsl/parser_impl_for_stmt_test.cc", "reader/wgsl/parser_impl_function_attribute_list_test.cc", diff --git a/src/tint/CMakeLists.txt b/src/tint/CMakeLists.txt index 288735a226..5c2f639594 100644 --- a/src/tint/CMakeLists.txt +++ b/src/tint/CMakeLists.txt @@ -961,11 +961,12 @@ if(TINT_BUILD_TESTS) reader/wgsl/parser_impl_depth_texture_test.cc reader/wgsl/parser_impl_element_count_expression_test.cc reader/wgsl/parser_impl_enable_directive_test.cc - reader/wgsl/parser_impl_external_texture_test.cc reader/wgsl/parser_impl_equality_expression_test.cc reader/wgsl/parser_impl_error_msg_test.cc reader/wgsl/parser_impl_error_resync_test.cc reader/wgsl/parser_impl_exclusive_or_expression_test.cc + reader/wgsl/parser_impl_expression_test.cc + reader/wgsl/parser_impl_external_texture_test.cc reader/wgsl/parser_impl_for_stmt_test.cc reader/wgsl/parser_impl_function_decl_test.cc reader/wgsl/parser_impl_function_attribute_list_test.cc diff --git a/src/tint/reader/wgsl/parser_impl.cc b/src/tint/reader/wgsl/parser_impl.cc index cf684d3d0b..dbcb2c7d92 100644 --- a/src/tint/reader/wgsl/parser_impl.cc +++ b/src/tint/reader/wgsl/parser_impl.cc @@ -2908,6 +2908,74 @@ Expect ParserImpl::expect_relational_expression_post_una return lhs; } +// expression +// : unary_expression bitwise_expression.post.unary_expression +// | unary_expression relational_expression.post.unary_expression +// | unary_expression relational_expression.post.unary_expression and_and +// relational_expression ( and_and relational_expression )* +// | unary_expression relational_expression.post.unary_expression or_or +// relational_expression ( or_or relational_expression )* +// +// Note, a `relational_expression` element was added to simplify many of the right sides +Maybe ParserImpl::maybe_expression() { + auto lhs = unary_expression(); + if (lhs.errored) { + return Failure::kErrored; + } + if (!lhs.matched) { + return Failure::kNoMatch; + } + + auto bitwise = bitwise_expression_post_unary_expression(lhs.value); + if (bitwise.errored) { + return Failure::kErrored; + } + if (bitwise.matched) { + return bitwise.value; + } + + auto relational = expect_relational_expression_post_unary_expression(lhs.value); + if (relational.errored) { + return Failure::kErrored; + } + auto* ret = relational.value; + + auto& t = peek(); + if (t.Is(Token::Type::kAndAnd) || t.Is(Token::Type::kOrOr)) { + ast::BinaryOp op = ast::BinaryOp::kNone; + if (t.Is(Token::Type::kAndAnd)) { + op = ast::BinaryOp::kLogicalAnd; + } else if (t.Is(Token::Type::kOrOr)) { + op = ast::BinaryOp::kLogicalOr; + } + + while (continue_parsing()) { + auto& n = peek(); + if (!n.Is(t.type())) { + if (n.Is(Token::Type::kAndAnd) || n.Is(Token::Type::kOrOr)) { + return add_error( + n.source(), std::string("mixing '") + std::string(t.to_name()) + "' and '" + + std::string(n.to_name()) + "' requires parenthesis"); + } + break; + } + next(); + + auto rhs = relational_expression(); + if (rhs.errored) { + return Failure::kErrored; + } + if (!rhs.matched) { + return add_error(peek(), std::string("unable to parse right side of ") + + std::string(t.to_name()) + " expression"); + } + + ret = create(t.source(), op, ret, rhs.value); + } + } + return ret; +} + // singular_expression // : primary_expression postfix_expr Maybe ParserImpl::singular_expression() { diff --git a/src/tint/reader/wgsl/parser_impl.h b/src/tint/reader/wgsl/parser_impl.h index d6d3cccc54..89bdb216a1 100644 --- a/src/tint/reader/wgsl/parser_impl.h +++ b/src/tint/reader/wgsl/parser_impl.h @@ -630,6 +630,9 @@ class ParserImpl { /// @param lhs the left side of the expression /// @returns the parsed expression or nullptr Expect expect_relational_expr(const ast::Expression* lhs); + /// Parses the `expression` grammar rule + /// @returns the parsed expression or nullptr + Maybe maybe_expression(); /// Parses the `relational_expression` grammar element /// @returns the parsed expression or nullptr Maybe relational_expression(); diff --git a/src/tint/reader/wgsl/parser_impl_expression_test.cc b/src/tint/reader/wgsl/parser_impl_expression_test.cc new file mode 100644 index 0000000000..2938e3c7b7 --- /dev/null +++ b/src/tint/reader/wgsl/parser_impl_expression_test.cc @@ -0,0 +1,254 @@ +// Copyright 2022 The Tint Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "src/tint/reader/wgsl/parser_impl_test_helper.h" + +namespace tint::reader::wgsl { +namespace { + +TEST_F(ParserImplTest, Expression_InvalidLHS) { + auto p = parser("if (a) {} || true"); + auto e = p->maybe_expression(); + EXPECT_FALSE(e.matched); + EXPECT_FALSE(e.errored); + EXPECT_FALSE(p->has_error()) << p->error(); + EXPECT_EQ(e.value, nullptr); +} + +TEST_F(ParserImplTest, Expression_Or_Parses) { + auto p = parser("a || true"); + auto e = p->maybe_expression(); + EXPECT_TRUE(e.matched); + EXPECT_FALSE(e.errored); + EXPECT_FALSE(p->has_error()) << p->error(); + ASSERT_NE(e.value, nullptr); + + EXPECT_EQ(e->source.range.begin.line, 1u); + EXPECT_EQ(e->source.range.begin.column, 3u); + EXPECT_EQ(e->source.range.end.line, 1u); + EXPECT_EQ(e->source.range.end.column, 5u); + + ASSERT_TRUE(e->Is()); + auto* rel = e->As(); + EXPECT_EQ(ast::BinaryOp::kLogicalOr, rel->op); + + ASSERT_TRUE(rel->lhs->Is()); + auto* ident = rel->lhs->As(); + EXPECT_EQ(ident->symbol, p->builder().Symbols().Get("a")); + + ASSERT_TRUE(rel->rhs->Is()); + ASSERT_TRUE(rel->rhs->As()->value); +} + +TEST_F(ParserImplTest, Expression_Or_Parses_Multiple) { + auto p = parser("a || true || b"); + auto e = p->maybe_expression(); + EXPECT_TRUE(e.matched); + EXPECT_FALSE(e.errored); + EXPECT_FALSE(p->has_error()) << p->error(); + ASSERT_NE(e.value, nullptr); + + ASSERT_TRUE(e->Is()); + // lhs: (a || true) + // rhs: b + auto* rel = e->As(); + EXPECT_EQ(ast::BinaryOp::kLogicalOr, rel->op); + + ASSERT_TRUE(rel->rhs->Is()); + auto* ident = rel->rhs->As(); + EXPECT_EQ(ident->symbol, p->builder().Symbols().Get("b")); + + ASSERT_TRUE(rel->lhs->Is()); + // lhs: a + // rhs: true + rel = rel->lhs->As(); + EXPECT_EQ(ast::BinaryOp::kLogicalOr, rel->op); + + ASSERT_TRUE(rel->lhs->Is()); + ident = rel->lhs->As(); + EXPECT_EQ(ident->symbol, p->builder().Symbols().Get("a")); + + ASSERT_TRUE(rel->rhs->Is()); + ASSERT_TRUE(rel->rhs->As()->value); +} + +TEST_F(ParserImplTest, Expression_Or_InvalidRHS) { + auto p = parser("true || if (a) {}"); + auto e = p->maybe_expression(); + EXPECT_FALSE(e.matched); + EXPECT_TRUE(e.errored); + EXPECT_EQ(e.value, nullptr); + EXPECT_TRUE(p->has_error()); + EXPECT_EQ(p->error(), "1:9: unable to parse right side of || expression"); +} + +TEST_F(ParserImplTest, Expression_And_Parses) { + auto p = parser("a && true"); + auto e = p->maybe_expression(); + EXPECT_TRUE(e.matched); + EXPECT_FALSE(e.errored); + EXPECT_FALSE(p->has_error()) << p->error(); + ASSERT_NE(e.value, nullptr); + + EXPECT_EQ(e->source.range.begin.line, 1u); + EXPECT_EQ(e->source.range.begin.column, 3u); + EXPECT_EQ(e->source.range.end.line, 1u); + EXPECT_EQ(e->source.range.end.column, 5u); + + ASSERT_TRUE(e->Is()); + auto* rel = e->As(); + EXPECT_EQ(ast::BinaryOp::kLogicalAnd, rel->op); + + ASSERT_TRUE(rel->lhs->Is()); + auto* ident = rel->lhs->As(); + EXPECT_EQ(ident->symbol, p->builder().Symbols().Get("a")); + + ASSERT_TRUE(rel->rhs->Is()); + ASSERT_TRUE(rel->rhs->As()->value); +} + +TEST_F(ParserImplTest, Expression_And_Parses_Multple) { + auto p = parser("a && true && b"); + auto e = p->maybe_expression(); + EXPECT_TRUE(e.matched); + EXPECT_FALSE(e.errored); + EXPECT_FALSE(p->has_error()) << p->error(); + ASSERT_NE(e.value, nullptr); + + ASSERT_TRUE(e->Is()); + // lhs: (a && true) + // rhs: b + auto* rel = e->As(); + EXPECT_EQ(ast::BinaryOp::kLogicalAnd, rel->op); + + ASSERT_TRUE(rel->rhs->Is()); + auto* ident = rel->rhs->As(); + EXPECT_EQ(ident->symbol, p->builder().Symbols().Get("b")); + + ASSERT_TRUE(rel->lhs->Is()); + // lhs: a + // rhs: true + rel = rel->lhs->As(); + ASSERT_TRUE(rel->lhs->Is()); + ident = rel->lhs->As(); + EXPECT_EQ(ident->symbol, p->builder().Symbols().Get("a")); + + ASSERT_TRUE(rel->rhs->Is()); + ASSERT_TRUE(rel->rhs->As()->value); +} + +TEST_F(ParserImplTest, Expression_And_InvalidRHS) { + auto p = parser("true && if (a) {}"); + auto e = p->maybe_expression(); + EXPECT_FALSE(e.matched); + EXPECT_TRUE(e.errored); + EXPECT_EQ(e.value, nullptr); + EXPECT_TRUE(p->has_error()); + EXPECT_EQ(p->error(), "1:9: unable to parse right side of && expression"); +} + +TEST_F(ParserImplTest, Expression_Mixing_OrWithAnd) { + auto p = parser("a && true || b"); + auto e = p->maybe_expression(); + EXPECT_FALSE(e.matched); + EXPECT_TRUE(e.errored); + EXPECT_EQ(e.value, nullptr); + EXPECT_TRUE(p->has_error()); + EXPECT_EQ(p->error(), "1:11: mixing '&&' and '||' requires parenthesis"); +} + +TEST_F(ParserImplTest, Expression_Mixing_AndWithOr) { + auto p = parser("a || true && b"); + auto e = p->maybe_expression(); + EXPECT_FALSE(e.matched); + EXPECT_TRUE(e.errored); + EXPECT_EQ(e.value, nullptr); + EXPECT_TRUE(p->has_error()); + EXPECT_EQ(p->error(), "1:11: mixing '||' and '&&' requires parenthesis"); +} + +TEST_F(ParserImplTest, Expression_Bitwise) { + auto p = parser("a & b"); + auto e = p->maybe_expression(); + EXPECT_TRUE(e.matched); + EXPECT_FALSE(e.errored); + EXPECT_FALSE(p->has_error()) << p->error(); + ASSERT_NE(e.value, nullptr); + + ASSERT_TRUE(e->Is()); + auto* rel = e->As(); + EXPECT_EQ(ast::BinaryOp::kAnd, rel->op); + + ASSERT_TRUE(rel->lhs->Is()); + auto* ident = rel->lhs->As(); + EXPECT_EQ(ident->symbol, p->builder().Symbols().Get("a")); + + ASSERT_TRUE(rel->rhs->Is()); + ident = rel->rhs->As(); + EXPECT_EQ(ident->symbol, p->builder().Symbols().Get("b")); +} + +TEST_F(ParserImplTest, Expression_Relational) { + auto p = parser("a <= b"); + auto e = p->maybe_expression(); + EXPECT_TRUE(e.matched); + EXPECT_FALSE(e.errored); + EXPECT_FALSE(p->has_error()) << p->error(); + ASSERT_NE(e.value, nullptr); + + ASSERT_TRUE(e->Is()); + auto* rel = e->As(); + EXPECT_EQ(ast::BinaryOp::kLessThanEqual, rel->op); + + ASSERT_TRUE(rel->lhs->Is()); + auto* ident = rel->lhs->As(); + EXPECT_EQ(ident->symbol, p->builder().Symbols().Get("a")); + + ASSERT_TRUE(rel->rhs->Is()); + ident = rel->rhs->As(); + EXPECT_EQ(ident->symbol, p->builder().Symbols().Get("b")); +} + +TEST_F(ParserImplTest, Expression_InvalidUnary) { + auto p = parser("!if || true"); + auto e = p->maybe_expression(); + EXPECT_FALSE(e.matched); + EXPECT_TRUE(e.errored); + EXPECT_EQ(e.value, nullptr); + EXPECT_TRUE(p->has_error()); + EXPECT_EQ(p->error(), "1:2: unable to parse right side of ! expression"); +} + +TEST_F(ParserImplTest, Expression_InvalidBitwise) { + auto p = parser("a & if"); + auto e = p->maybe_expression(); + EXPECT_FALSE(e.matched); + EXPECT_TRUE(e.errored); + EXPECT_EQ(e.value, nullptr); + EXPECT_TRUE(p->has_error()); + EXPECT_EQ(p->error(), "1:5: unable to parse right side of & expression"); +} + +TEST_F(ParserImplTest, Expression_InvalidRelational) { + auto p = parser("a <= if"); + auto e = p->maybe_expression(); + EXPECT_FALSE(e.matched); + EXPECT_TRUE(e.errored); + EXPECT_EQ(e.value, nullptr); + EXPECT_TRUE(p->has_error()); + EXPECT_EQ(p->error(), "1:6: unable to parse right side of <= expression"); +} + +} // namespace +} // namespace tint::reader::wgsl