From 00aa7ef462c5cbc450f081a97dc68dddb7b2425a Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Tue, 18 Oct 2022 23:57:25 +0000 Subject: [PATCH] tint/reader/wgsl: Better diagnostics for missing parentheses This change required removing the `&&` splitting for `a & b && c` which never valid WGSL (right now). Fixed: tint:1658 Change-Id: Ideb9f1aa9cf9b9b1054a6fc65860106dc072a9dc Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/105820 Commit-Queue: Ben Clayton Kokoro: Kokoro Reviewed-by: David Neto Reviewed-by: Dan Sinclair --- src/tint/ast/binary_expression.h | 48 +++- src/tint/reader/wgsl/parser_impl.cc | 226 +++++++++--------- .../parser_impl_bitwise_expression_test.cc | 90 +------ .../wgsl/parser_impl_expression_test.cc | 122 +++++++++- ...arser_impl_struct_member_attribute_test.cc | 2 +- src/tint/reader/wgsl/token.h | 31 ++- 6 files changed, 317 insertions(+), 202 deletions(-) diff --git a/src/tint/ast/binary_expression.h b/src/tint/ast/binary_expression.h index cdc5960f3c..323ab4840f 100644 --- a/src/tint/ast/binary_expression.h +++ b/src/tint/ast/binary_expression.h @@ -248,7 +248,53 @@ constexpr const char* FriendlyName(BinaryOp op) { case BinaryOp::kModulo: return "modulo"; } - return "INVALID"; + return ""; +} + +/// @returns the WGSL operator of the BinaryOp +/// @param op the BinaryOp +constexpr const char* Operator(BinaryOp op) { + switch (op) { + case BinaryOp::kAnd: + return "&"; + case BinaryOp::kOr: + return "|"; + case BinaryOp::kXor: + return "^"; + case BinaryOp::kLogicalAnd: + return "&&"; + case BinaryOp::kLogicalOr: + return "||"; + case BinaryOp::kEqual: + return "=="; + case BinaryOp::kNotEqual: + return "!="; + case BinaryOp::kLessThan: + return "<"; + case BinaryOp::kGreaterThan: + return ">"; + case BinaryOp::kLessThanEqual: + return "<="; + case BinaryOp::kGreaterThanEqual: + return ">="; + case BinaryOp::kShiftLeft: + return "<<"; + case BinaryOp::kShiftRight: + return ">>"; + case BinaryOp::kAdd: + return "+"; + case BinaryOp::kSubtract: + return "-"; + case BinaryOp::kMultiply: + return "*"; + case BinaryOp::kDivide: + return "/"; + case BinaryOp::kModulo: + return "%"; + default: + break; + } + return ""; } /// @param out the std::ostream to write to diff --git a/src/tint/reader/wgsl/parser_impl.cc b/src/tint/reader/wgsl/parser_impl.cc index 8d6040d9d8..d8d799415f 100644 --- a/src/tint/reader/wgsl/parser_impl.cc +++ b/src/tint/reader/wgsl/parser_impl.cc @@ -2662,43 +2662,24 @@ Expect ParserImpl::expect_argument_expression_list( Maybe ParserImpl::bitwise_expression_post_unary_expression( const ast::Expression* lhs) { auto& t = peek(); - if (!t.Is(Token::Type::kAnd) && !t.Is(Token::Type::kOr) && !t.Is(Token::Type::kXor)) { - return Failure::kNoMatch; - } ast::BinaryOp op = ast::BinaryOp::kXor; - if (t.Is(Token::Type::kAnd)) { - op = ast::BinaryOp::kAnd; - } else if (t.Is(Token::Type::kOr)) { - op = ast::BinaryOp::kOr; + switch (t.type()) { + case Token::Type::kAnd: + op = ast::BinaryOp::kAnd; + break; + case Token::Type::kOr: + op = ast::BinaryOp::kOr; + break; + case Token::Type::kXor: + op = ast::BinaryOp::kXor; + break; + default: + return Failure::kNoMatch; } + next(); // Consume t while (continue_parsing()) { - auto& n = peek(); - // Handle the case of `a & b &&c` where `&c` is a unary_expression - bool split = false; - if (op == ast::BinaryOp::kAnd && n.Is(Token::Type::kAndAnd)) { - next(); - split_token(Token::Type::kAnd, Token::Type::kAnd); - split = true; - } - - if (!n.Is(t.type())) { - if (n.Is(Token::Type::kAnd) || n.Is(Token::Type::kOr) || n.Is(Token::Type::kXor)) { - return add_error(n.source(), std::string("mixing '") + std::string(t.to_name()) + - "' and '" + std::string(n.to_name()) + - "' requires parenthesis"); - } - - return lhs; - } - // If forced to split an `&&` then we've already done the `next` above which consumes - // the `&`. The type check above will always fail because we only split if already consuming - // a `&` operator. - if (!split) { - next(); - } - auto rhs = unary_expression(); if (rhs.errored) { return Failure::kErrored; @@ -2709,6 +2690,10 @@ Maybe ParserImpl::bitwise_expression_post_unary_expressi } lhs = create(t.source(), op, lhs, rhs.value); + + if (!match(t.type())) { + return lhs; + } } return Failure::kErrored; } @@ -2949,37 +2934,45 @@ Expect ParserImpl::expect_relational_expression_post_una } lhs = lhs_result.value; - auto& t = peek(); - if (match(Token::Type::kEqualEqual) || match(Token::Type::kGreaterThan) || - match(Token::Type::kGreaterThanEqual) || match(Token::Type::kLessThan) || - match(Token::Type::kLessThanEqual) || match(Token::Type::kNotEqual)) { - ast::BinaryOp op = ast::BinaryOp::kNone; - if (t.Is(Token::Type::kLessThan)) { - op = ast::BinaryOp::kLessThan; - } else if (t.Is(Token::Type::kGreaterThan)) { - op = ast::BinaryOp::kGreaterThan; - } else if (t.Is(Token::Type::kLessThanEqual)) { - op = ast::BinaryOp::kLessThanEqual; - } else if (t.Is(Token::Type::kGreaterThanEqual)) { - op = ast::BinaryOp::kGreaterThanEqual; - } else if (t.Is(Token::Type::kEqualEqual)) { - op = ast::BinaryOp::kEqual; - } else if (t.Is(Token::Type::kNotEqual)) { - op = ast::BinaryOp::kNotEqual; - } + auto& tok_op = peek(); - auto& next = peek(); - auto rhs = shift_expression(); - if (rhs.errored) { - return Failure::kErrored; - } - if (!rhs.matched) { - return add_error(next, std::string("unable to parse right side of ") + - std::string(t.to_name()) + " expression"); - } - lhs = create(t.source(), op, lhs, rhs.value); + ast::BinaryOp op = ast::BinaryOp::kNone; + switch (tok_op.type()) { + case Token::Type::kLessThan: + op = ast::BinaryOp::kLessThan; + break; + case Token::Type::kGreaterThan: + op = ast::BinaryOp::kGreaterThan; + break; + case Token::Type::kLessThanEqual: + op = ast::BinaryOp::kLessThanEqual; + break; + case Token::Type::kGreaterThanEqual: + op = ast::BinaryOp::kGreaterThanEqual; + break; + case Token::Type::kEqualEqual: + op = ast::BinaryOp::kEqual; + break; + case Token::Type::kNotEqual: + op = ast::BinaryOp::kNotEqual; + break; + default: + return lhs; } - return lhs; + + next(); // consume tok_op + + auto& tok_rhs = peek(); + auto rhs = shift_expression(); + if (rhs.errored) { + return Failure::kErrored; + } + if (!rhs.matched) { + return add_error(tok_rhs, std::string("unable to parse right side of ") + + std::string(tok_op.to_name()) + " expression"); + } + + return create(tok_op.source(), op, lhs, rhs.value); } // expression @@ -2992,62 +2985,75 @@ Expect ParserImpl::expect_relational_expression_post_una // // Note, a `relational_expression` element was added to simplify many of the right sides Maybe ParserImpl::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; + auto expr = [&]() -> Maybe { + auto lhs = unary_expression(); + if (lhs.errored) { + return Failure::kErrored; + } + if (!lhs.matched) { + return Failure::kNoMatch; } - 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 bitwise = bitwise_expression_post_unary_expression(lhs.value); + if (bitwise.errored) { + return Failure::kErrored; + } + if (bitwise.matched) { + return bitwise.value; + } - auto rhs = relational_expression(); - if (rhs.errored) { + 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())) { + 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; + }(); + + if (expr.matched) { + // Note, expression is greedy an will consume all the operators of the same type + // so, `a & a & a` would all be consumed above. If you see any binary operator + // after this then it _must_ be a different one, and hence an error. + if (auto* lhs = expr->As()) { + if (auto& n = peek(); n.IsBinaryOperator()) { + auto source = Source::Combine(expr->source, n.source()); + add_error(source, std::string("mixing '") + ast::Operator(lhs->op) + "' and '" + + std::string(n.to_name()) + "' requires parenthesis"); 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; + + return expr; } // singular_expression diff --git a/src/tint/reader/wgsl/parser_impl_bitwise_expression_test.cc b/src/tint/reader/wgsl/parser_impl_bitwise_expression_test.cc index a5f675ca49..9e95a3c14f 100644 --- a/src/tint/reader/wgsl/parser_impl_bitwise_expression_test.cc +++ b/src/tint/reader/wgsl/parser_impl_bitwise_expression_test.cc @@ -87,28 +87,6 @@ TEST_F(ParserImplTest, BitwiseExpr_Or_Parses_Multiple) { ASSERT_TRUE(rel->rhs->As()->value); } -TEST_F(ParserImplTest, BitwiseExpr_Or_MixedWithAnd_Invalid) { - auto p = parser("a | b & c"); - auto lhs = p->unary_expression(); - auto e = p->bitwise_expression_post_unary_expression(lhs.value); - EXPECT_FALSE(e.matched); - EXPECT_TRUE(e.errored); - EXPECT_EQ(e.value, nullptr); - EXPECT_TRUE(p->has_error()); - EXPECT_EQ(p->error(), "1:7: mixing '|' and '&' requires parenthesis"); -} - -TEST_F(ParserImplTest, BitwiseExpr_Or_MixedWithXor_Invalid) { - auto p = parser("a | b ^ c"); - auto lhs = p->unary_expression(); - auto e = p->bitwise_expression_post_unary_expression(lhs.value); - EXPECT_FALSE(e.matched); - EXPECT_TRUE(e.errored); - EXPECT_EQ(e.value, nullptr); - EXPECT_TRUE(p->has_error()); - EXPECT_EQ(p->error(), "1:7: mixing '|' and '^' requires parenthesis"); -} - TEST_F(ParserImplTest, BitwiseExpr_Or_InvalidRHS) { auto p = parser("true | if (a) {}"); auto lhs = p->unary_expression(); @@ -179,28 +157,6 @@ TEST_F(ParserImplTest, BitwiseExpr_Xor_Parses_Multiple) { ASSERT_TRUE(rel->rhs->As()->value); } -TEST_F(ParserImplTest, BitwiseExpr_Xor_MixedWithOr_Invalid) { - auto p = parser("a ^ b | c"); - auto lhs = p->unary_expression(); - auto e = p->bitwise_expression_post_unary_expression(lhs.value); - EXPECT_FALSE(e.matched); - EXPECT_TRUE(e.errored); - EXPECT_EQ(e.value, nullptr); - EXPECT_TRUE(p->has_error()); - EXPECT_EQ(p->error(), "1:7: mixing '^' and '|' requires parenthesis"); -} - -TEST_F(ParserImplTest, BitwiseExpr_Xor_MixedWithAnd_Invalid) { - auto p = parser("a ^ b & c"); - auto lhs = p->unary_expression(); - auto e = p->bitwise_expression_post_unary_expression(lhs.value); - EXPECT_FALSE(e.matched); - EXPECT_TRUE(e.errored); - EXPECT_EQ(e.value, nullptr); - EXPECT_TRUE(p->has_error()); - EXPECT_EQ(p->error(), "1:7: mixing '^' and '&' requires parenthesis"); -} - TEST_F(ParserImplTest, BitwiseExpr_Xor_InvalidRHS) { auto p = parser("true ^ if (a) {}"); auto lhs = p->unary_expression(); @@ -275,59 +231,21 @@ TEST_F(ParserImplTest, BitwiseExpr_And_Parses_AndAnd) { auto p = parser("a & true &&b"); auto lhs = p->unary_expression(); auto e = p->bitwise_expression_post_unary_expression(lhs.value); + // bitwise_expression_post_unary_expression returns before parsing '&&' + EXPECT_TRUE(e.matched); EXPECT_FALSE(e.errored); EXPECT_FALSE(p->has_error()) << p->error(); ASSERT_NE(e.value, nullptr); - // lhs: (a & true) - // rhs: &b + // lhs: a + // rhs: true ASSERT_TRUE(e->Is()); auto* rel = e->As(); EXPECT_EQ(ast::BinaryOp::kAnd, rel->op); - ASSERT_TRUE(rel->rhs->Is()); - auto* unary = rel->rhs->As(); - EXPECT_EQ(ast::UnaryOp::kAddressOf, unary->op); - - ASSERT_TRUE(unary->expr->Is()); - auto* ident = unary->expr->As(); - EXPECT_EQ(ident->symbol, p->builder().Symbols().Register("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().Register("a")); - ASSERT_TRUE(rel->rhs->Is()); - ASSERT_TRUE(rel->rhs->As()->value); -} - -TEST_F(ParserImplTest, BitwiseExpr_And_MixedWithOr_Invalid) { - auto p = parser("a & b | c"); - auto lhs = p->unary_expression(); - auto e = p->bitwise_expression_post_unary_expression(lhs.value); - EXPECT_FALSE(e.matched); - EXPECT_TRUE(e.errored); - EXPECT_EQ(e.value, nullptr); - EXPECT_TRUE(p->has_error()); - EXPECT_EQ(p->error(), "1:7: mixing '&' and '|' requires parenthesis"); -} - -TEST_F(ParserImplTest, BitwiseExpr_And_MixedWithXor_Invalid) { - auto p = parser("a & b ^ c"); - auto lhs = p->unary_expression(); - auto e = p->bitwise_expression_post_unary_expression(lhs.value); - EXPECT_FALSE(e.matched); - EXPECT_TRUE(e.errored); - EXPECT_EQ(e.value, nullptr); - EXPECT_TRUE(p->has_error()); - EXPECT_EQ(p->error(), "1:7: mixing '&' and '^' requires parenthesis"); } TEST_F(ParserImplTest, BitwiseExpr_And_InvalidRHS) { diff --git a/src/tint/reader/wgsl/parser_impl_expression_test.cc b/src/tint/reader/wgsl/parser_impl_expression_test.cc index 0adbeeff01..e3e53fc8f1 100644 --- a/src/tint/reader/wgsl/parser_impl_expression_test.cc +++ b/src/tint/reader/wgsl/parser_impl_expression_test.cc @@ -165,7 +165,7 @@ TEST_F(ParserImplTest, Expression_Mixing_OrWithAnd) { EXPECT_TRUE(e.errored); EXPECT_EQ(e.value, nullptr); EXPECT_TRUE(p->has_error()); - EXPECT_EQ(p->error(), "1:11: mixing '&&' and '||' requires parenthesis"); + EXPECT_EQ(p->error(), "1:3: mixing '&&' and '||' requires parenthesis"); } TEST_F(ParserImplTest, Expression_Mixing_AndWithOr) { @@ -175,7 +175,7 @@ TEST_F(ParserImplTest, Expression_Mixing_AndWithOr) { EXPECT_TRUE(e.errored); EXPECT_EQ(e.value, nullptr); EXPECT_TRUE(p->has_error()); - EXPECT_EQ(p->error(), "1:11: mixing '||' and '&&' requires parenthesis"); + EXPECT_EQ(p->error(), "1:3: mixing '||' and '&&' requires parenthesis"); } TEST_F(ParserImplTest, Expression_Bitwise) { @@ -250,5 +250,123 @@ TEST_F(ParserImplTest, Expression_InvalidRelational) { EXPECT_EQ(p->error(), "1:6: unable to parse right side of <= expression"); } +namespace mixing_binary_ops { + +struct BinaryOperatorInfo { + // A uint64_t with a single bit assigned that uniquely identifies the binary-op. + uint64_t bit; + // The WGSL operator symbol. Example: '<=' + const char* symbol; + // A bit mask of all operators that can immediately follow the RHS of this operator without + // requiring parentheses. In other words, `can_follow_without_paren` is the full set of + // operators that can substitute `` in the WGSL: + // `expr_a expr_b expr_c` + // without requiring additional parentheses. + uint64_t can_follow_without_paren; +}; + +// Each binary operator is given a unique bit in a uint64_t +static constexpr uint64_t kOpMul = static_cast(1) << 0; +static constexpr uint64_t kOpDiv = static_cast(1) << 1; +static constexpr uint64_t kOpMod = static_cast(1) << 2; +static constexpr uint64_t kOpAdd = static_cast(1) << 3; +static constexpr uint64_t kOpSub = static_cast(1) << 4; +static constexpr uint64_t kOpBAnd = static_cast(1) << 5; +static constexpr uint64_t kOpBOr = static_cast(1) << 6; +static constexpr uint64_t kOpBXor = static_cast(1) << 7; +static constexpr uint64_t kOpShl = static_cast(1) << 8; +static constexpr uint64_t kOpShr = static_cast(1) << 9; +static constexpr uint64_t kOpLt = static_cast(1) << 10; +static constexpr uint64_t kOpGt = static_cast(1) << 11; +static constexpr uint64_t kOpLe = static_cast(1) << 12; +static constexpr uint64_t kOpGe = static_cast(1) << 13; +static constexpr uint64_t kOpEq = static_cast(1) << 14; +static constexpr uint64_t kOpNe = static_cast(1) << 15; +static constexpr uint64_t kOpLAnd = static_cast(1) << 16; +static constexpr uint64_t kOpLOr = static_cast(1) << 17; + +// Bit mask for the binary operator groups +static constexpr uint64_t kMultiplicative = kOpMul | kOpDiv | kOpMod; +static constexpr uint64_t kAdditive = kOpAdd | kOpSub; +static constexpr uint64_t kShift = kOpShl | kOpShr; +static constexpr uint64_t kRelational = kOpLt | kOpGt | kOpLe | kOpGe | kOpEq | kOpNe; +static constexpr uint64_t kLogical = kOpLAnd | kOpLOr; + +// The binary operator table +static constexpr const BinaryOperatorInfo kBinaryOperators[] = { + // multiplicative + {kOpMul, "*", kLogical | kRelational | kAdditive | kMultiplicative}, + {kOpDiv, "/", kLogical | kRelational | kAdditive | kMultiplicative}, + {kOpMod, "%", kLogical | kRelational | kAdditive | kMultiplicative}, + // additive + {kOpAdd, "+", kLogical | kRelational | kAdditive | kMultiplicative}, + {kOpSub, "-", kLogical | kRelational | kAdditive | kMultiplicative}, + // bitwise + {kOpBAnd, "&", kOpBAnd}, + {kOpBOr, "|", kOpBOr}, + {kOpBXor, "^", kOpBXor}, + // shift + {kOpShl, "<<", kLogical | kRelational}, + {kOpShr, ">>", kLogical | kRelational}, + // relational + {kOpLt, "<", kLogical | kShift | kAdditive | kMultiplicative}, + {kOpGt, ">", kLogical | kShift | kAdditive | kMultiplicative}, + {kOpLe, "<=", kLogical | kShift | kAdditive | kMultiplicative}, + {kOpGe, ">=", kLogical | kShift | kAdditive | kMultiplicative}, + {kOpEq, "==", kLogical | kShift | kAdditive | kMultiplicative}, + {kOpNe, "!=", kLogical | kShift | kAdditive | kMultiplicative}, + // logical + {kOpLAnd, "&&", kOpLAnd | kRelational | kShift | kAdditive | kMultiplicative}, + {kOpLOr, "||", kOpLOr | kRelational | kShift | kAdditive | kMultiplicative}, +}; + +struct Case { + BinaryOperatorInfo lhs_op; + BinaryOperatorInfo rhs_op; + bool should_parse; +}; + +static std::ostream& operator<<(std::ostream& o, const Case& c) { + return o << "a " << c.lhs_op.symbol << " b " << c.rhs_op.symbol << " c "; +} + +static std::vector Cases() { + std::vector out; + for (auto& lhs_op : kBinaryOperators) { + for (auto& rhs_op : kBinaryOperators) { + bool should_parse = lhs_op.can_follow_without_paren & rhs_op.bit; + out.push_back({lhs_op, rhs_op, should_parse}); + } + } + return out; +} + +using ParserImplMixedBinaryOpTest = ParserImplTestWithParam; + +TEST_P(ParserImplMixedBinaryOpTest, Test) { + std::stringstream wgsl; + wgsl << GetParam(); + auto p = parser(wgsl.str()); + auto e = p->expression(); + if (GetParam().should_parse) { + ASSERT_TRUE(e.matched) << e.errored; + EXPECT_TRUE(e->Is()); + } else { + EXPECT_FALSE(e.matched); + EXPECT_TRUE(e.errored); + EXPECT_EQ(e.value, nullptr); + EXPECT_TRUE(p->has_error()); + std::stringstream expected; + expected << "1:3: mixing '" << GetParam().lhs_op.symbol << "' and '" + << GetParam().rhs_op.symbol << "' requires parenthesis"; + EXPECT_EQ(p->error(), expected.str()); + } +} +INSTANTIATE_TEST_SUITE_P(ParserImplMixedBinaryOpTest, + ParserImplMixedBinaryOpTest, + testing::ValuesIn(Cases())); + +} // namespace mixing_binary_ops + } // namespace } // namespace tint::reader::wgsl diff --git a/src/tint/reader/wgsl/parser_impl_struct_member_attribute_test.cc b/src/tint/reader/wgsl/parser_impl_struct_member_attribute_test.cc index 80376d83f9..7abdcd5858 100644 --- a/src/tint/reader/wgsl/parser_impl_struct_member_attribute_test.cc +++ b/src/tint/reader/wgsl/parser_impl_struct_member_attribute_test.cc @@ -220,7 +220,7 @@ TEST_F(ParserImplTest, Attribute_Align_ExpressionInvalid) { EXPECT_EQ(attr.value, nullptr); EXPECT_TRUE(p->has_error()); - EXPECT_EQ(p->error(), "1:13: expected ')' for align attribute"); + EXPECT_EQ(p->error(), "1:9: mixing '+' and '<<' requires parenthesis"); } } // namespace diff --git a/src/tint/reader/wgsl/token.h b/src/tint/reader/wgsl/token.h index 4cf9aad1b1..3f92f48021 100644 --- a/src/tint/reader/wgsl/token.h +++ b/src/tint/reader/wgsl/token.h @@ -379,8 +379,35 @@ class Token { /// @returns true if the token can be split during parse into component tokens bool IsSplittable() const { - return Is(Token::Type::kShiftRight) || Is(Token::Type::kGreaterThanEqual) || - Is(Token::Type::kAndAnd) || Is(Token::Type::kMinusMinus); + return Is(Type::kShiftRight) || Is(Type::kGreaterThanEqual) || Is(Type::kAndAnd) || + Is(Type::kMinusMinus); + } + + /// @returns true if the token is a binary operator + bool IsBinaryOperator() const { + switch (type_) { + case Type::kAnd: + case Type::kAndAnd: + case Type::kEqualEqual: + case Type::kForwardSlash: + case Type::kGreaterThan: + case Type::kGreaterThanEqual: + case Type::kLessThan: + case Type::kLessThanEqual: + case Type::kMinus: + case Type::kMod: + case Type::kNotEqual: + case Type::kOr: + case Type::kOrOr: + case Type::kPlus: + case Type::kShiftLeft: + case Type::kShiftRight: + case Type::kStar: + case Type::kXor: + return true; + default: + return false; + } } /// @returns the source information for this token