diff --git a/src/tint/reader/wgsl/parser_impl.cc b/src/tint/reader/wgsl/parser_impl.cc index 6ca78d6899..87dc78e604 100644 --- a/src/tint/reader/wgsl/parser_impl.cc +++ b/src/tint/reader/wgsl/parser_impl.cc @@ -3167,6 +3167,7 @@ Maybe ParserImpl::attribute() { if (val.errored) { return Failure::kErrored; } + match(Token::Type::kComma); return create(t.source(), val.value); }); @@ -3179,6 +3180,7 @@ Maybe ParserImpl::attribute() { if (val.errored) { return Failure::kErrored; } + match(Token::Type::kComma); return create(t.source(), val.value); }); @@ -3191,6 +3193,7 @@ Maybe ParserImpl::attribute() { if (val.errored) { return Failure::kErrored; } + match(Token::Type::kComma); return create(t.source(), val.value); }); @@ -3213,15 +3216,19 @@ Maybe ParserImpl::attribute() { } if (match(Token::Type::kComma)) { - auto sampling_tok = next(); - if (sampling_tok == "center") { - sampling = ast::InterpolationSampling::kCenter; - } else if (sampling_tok == "centroid") { - sampling = ast::InterpolationSampling::kCentroid; - } else if (sampling_tok == "sample") { - sampling = ast::InterpolationSampling::kSample; - } else { - return add_error(sampling_tok, "invalid interpolation sampling"); + if (!peek_is(Token::Type::kParenRight)) { + auto sampling_tok = next(); + if (sampling_tok == "center") { + sampling = ast::InterpolationSampling::kCenter; + } else if (sampling_tok == "centroid") { + sampling = ast::InterpolationSampling::kCentroid; + } else if (sampling_tok == "sample") { + sampling = ast::InterpolationSampling::kSample; + } else { + return add_error(sampling_tok, "invalid interpolation sampling"); + } + + match(Token::Type::kComma); } } @@ -3240,6 +3247,7 @@ Maybe ParserImpl::attribute() { return Failure::kErrored; } + match(Token::Type::kComma); return create(t.source(), builtin.value); }); } @@ -3259,22 +3267,28 @@ Maybe ParserImpl::attribute() { x = std::move(expr.value); if (match(Token::Type::kComma)) { - expr = primary_expression(); - if (expr.errored) { - return Failure::kErrored; - } else if (!expr.matched) { - return add_error(peek(), "expected workgroup_size y parameter"); - } - y = std::move(expr.value); - - if (match(Token::Type::kComma)) { + if (!peek_is(Token::Type::kParenRight)) { expr = primary_expression(); if (expr.errored) { return Failure::kErrored; } else if (!expr.matched) { - return add_error(peek(), "expected workgroup_size z parameter"); + return add_error(peek(), "expected workgroup_size y parameter"); + } + y = std::move(expr.value); + + if (match(Token::Type::kComma)) { + if (!peek_is(Token::Type::kParenRight)) { + expr = primary_expression(); + if (expr.errored) { + return Failure::kErrored; + } else if (!expr.matched) { + return add_error(peek(), "expected workgroup_size z parameter"); + } + z = std::move(expr.value); + + match(Token::Type::kComma); + } } - z = std::move(expr.value); } } @@ -3326,6 +3340,7 @@ Maybe ParserImpl::attribute() { if (val.errored) { return Failure::kErrored; } + match(Token::Type::kComma); return create(t.source(), val.value); }); @@ -3338,6 +3353,7 @@ Maybe ParserImpl::attribute() { if (val.errored) { return Failure::kErrored; } + match(Token::Type::kComma); return create(t.source(), val.value); }); @@ -3350,6 +3366,7 @@ Maybe ParserImpl::attribute() { if (val.errored) { return Failure::kErrored; } + match(Token::Type::kComma); return create(t.source(), val.value); }); 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 453e549fdb..6a825de3f7 100644 --- a/src/tint/reader/wgsl/parser_impl_error_msg_test.cc +++ b/src/tint/reader/wgsl/parser_impl_error_msg_test.cc @@ -357,18 +357,18 @@ TEST_F(ParserImplErrorTest, FunctionDeclWorkgroupSizeXInvalid) { } TEST_F(ParserImplErrorTest, FunctionDeclWorkgroupSizeYInvalid) { - EXPECT("@workgroup_size(1, ) fn f() {}", + EXPECT("@workgroup_size(1, fn) fn f() {}", R"(test.wgsl:1:20 error: expected workgroup_size y parameter -@workgroup_size(1, ) fn f() {} - ^ +@workgroup_size(1, fn) fn f() {} + ^^ )"); } TEST_F(ParserImplErrorTest, FunctionDeclWorkgroupSizeZInvalid) { - EXPECT("@workgroup_size(1, 2, ) fn f() {}", + EXPECT("@workgroup_size(1, 2, fn) fn f() {}", R"(test.wgsl:1:23 error: expected workgroup_size z parameter -@workgroup_size(1, 2, ) fn f() {} - ^ +@workgroup_size(1, 2, fn) fn f() {} + ^^ )"); } diff --git a/src/tint/reader/wgsl/parser_impl_function_attribute_test.cc b/src/tint/reader/wgsl/parser_impl_function_attribute_test.cc index a32e140797..8e56ee7952 100644 --- a/src/tint/reader/wgsl/parser_impl_function_attribute_test.cc +++ b/src/tint/reader/wgsl/parser_impl_function_attribute_test.cc @@ -41,6 +41,38 @@ TEST_F(ParserImplTest, Attribute_Workgroup) { EXPECT_EQ(values[2], nullptr); } +TEST_F(ParserImplTest, Attribute_Workgroup_1Param_TrailingComma) { + auto p = parser("workgroup_size(4,)"); + auto attr = p->attribute(); + EXPECT_TRUE(attr.matched); + EXPECT_FALSE(attr.errored); + ASSERT_NE(attr.value, nullptr) << p->error(); + ASSERT_FALSE(p->has_error()); + auto* func_attr = attr.value->As(); + ASSERT_NE(func_attr, nullptr); + ASSERT_TRUE(func_attr->Is()); + + auto values = func_attr->As()->Values(); + + ASSERT_TRUE(values[0]->Is()); + EXPECT_EQ(values[0]->As()->value, 4); + EXPECT_EQ(values[0]->As()->suffix, + ast::IntLiteralExpression::Suffix::kNone); + + EXPECT_EQ(values[1], nullptr); + EXPECT_EQ(values[2], nullptr); +} + +TEST_F(ParserImplTest, Attribute_Workgroup_1Param_TrailingComma_Double) { + auto p = parser("workgroup_size(4,,)"); + auto attr = p->attribute(); + EXPECT_FALSE(attr.matched); + EXPECT_TRUE(attr.errored); + EXPECT_EQ(attr.value, nullptr); + EXPECT_TRUE(p->has_error()); + EXPECT_EQ(p->error(), "1:18: expected workgroup_size y parameter"); +} + TEST_F(ParserImplTest, Attribute_Workgroup_2Param) { auto p = parser("workgroup_size(4, 5)"); auto attr = p->attribute(); @@ -67,6 +99,42 @@ TEST_F(ParserImplTest, Attribute_Workgroup_2Param) { EXPECT_EQ(values[2], nullptr); } +TEST_F(ParserImplTest, Attribute_Workgroup_2Param_TrailingComma) { + auto p = parser("workgroup_size(4, 5,)"); + auto attr = p->attribute(); + EXPECT_TRUE(attr.matched); + EXPECT_FALSE(attr.errored); + ASSERT_NE(attr.value, nullptr) << p->error(); + ASSERT_FALSE(p->has_error()); + auto* func_attr = attr.value->As(); + ASSERT_NE(func_attr, nullptr) << p->error(); + ASSERT_TRUE(func_attr->Is()); + + auto values = func_attr->As()->Values(); + + ASSERT_TRUE(values[0]->Is()); + EXPECT_EQ(values[0]->As()->value, 4); + EXPECT_EQ(values[0]->As()->suffix, + ast::IntLiteralExpression::Suffix::kNone); + + ASSERT_TRUE(values[1]->Is()); + EXPECT_EQ(values[1]->As()->value, 5); + EXPECT_EQ(values[1]->As()->suffix, + ast::IntLiteralExpression::Suffix::kNone); + + EXPECT_EQ(values[2], nullptr); +} + +TEST_F(ParserImplTest, Attribute_Workgroup21Param_TrailingComma_Double) { + auto p = parser("workgroup_size(4,5,,)"); + auto attr = p->attribute(); + EXPECT_FALSE(attr.matched); + EXPECT_TRUE(attr.errored); + EXPECT_EQ(attr.value, nullptr); + EXPECT_TRUE(p->has_error()); + EXPECT_EQ(p->error(), "1:20: expected workgroup_size z parameter"); +} + TEST_F(ParserImplTest, Attribute_Workgroup_3Param) { auto p = parser("workgroup_size(4, 5, 6)"); auto attr = p->attribute(); @@ -96,6 +164,35 @@ TEST_F(ParserImplTest, Attribute_Workgroup_3Param) { ast::IntLiteralExpression::Suffix::kNone); } +TEST_F(ParserImplTest, Attribute_Workgroup_3Param_TrailingComma) { + auto p = parser("workgroup_size(4, 5, 6,)"); + auto attr = p->attribute(); + EXPECT_TRUE(attr.matched); + EXPECT_FALSE(attr.errored); + ASSERT_NE(attr.value, nullptr) << p->error(); + ASSERT_FALSE(p->has_error()); + auto* func_attr = attr.value->As(); + ASSERT_NE(func_attr, nullptr); + ASSERT_TRUE(func_attr->Is()); + + auto values = func_attr->As()->Values(); + + ASSERT_TRUE(values[0]->Is()); + EXPECT_EQ(values[0]->As()->value, 4); + EXPECT_EQ(values[0]->As()->suffix, + ast::IntLiteralExpression::Suffix::kNone); + + ASSERT_TRUE(values[1]->Is()); + EXPECT_EQ(values[1]->As()->value, 5); + EXPECT_EQ(values[1]->As()->suffix, + ast::IntLiteralExpression::Suffix::kNone); + + ASSERT_TRUE(values[2]->Is()); + EXPECT_EQ(values[2]->As()->value, 6); + EXPECT_EQ(values[2]->As()->suffix, + ast::IntLiteralExpression::Suffix::kNone); +} + TEST_F(ParserImplTest, Attribute_Workgroup_WithIdent) { auto p = parser("workgroup_size(4, height)"); auto attr = p->attribute(); @@ -129,7 +226,7 @@ TEST_F(ParserImplTest, Attribute_Workgroup_TooManyValues) { EXPECT_TRUE(attr.errored); EXPECT_EQ(attr.value, nullptr); EXPECT_TRUE(p->has_error()); - EXPECT_EQ(p->error(), "1:23: expected ')' for workgroup_size attribute"); + EXPECT_EQ(p->error(), "1:25: expected ')' for workgroup_size attribute"); } TEST_F(ParserImplTest, Attribute_Workgroup_MissingLeftParam) { @@ -202,16 +299,6 @@ TEST_F(ParserImplTest, Attribute_Workgroup_Missing_Z_Comma) { EXPECT_EQ(p->error(), "1:21: expected ')' for workgroup_size attribute"); } -TEST_F(ParserImplTest, Attribute_Workgroup_Missing_Z_Value) { - auto p = parser("workgroup_size(1, 2, )"); - auto attr = p->attribute(); - EXPECT_FALSE(attr.matched); - EXPECT_TRUE(attr.errored); - EXPECT_EQ(attr.value, nullptr); - EXPECT_TRUE(p->has_error()); - EXPECT_EQ(p->error(), "1:22: expected workgroup_size z parameter"); -} - // TODO(crbug.com/tint/1503): Remove when @stage is removed TEST_F(ParserImplTest, Attribute_Stage) { auto p = parser("stage(compute)"); diff --git a/src/tint/reader/wgsl/parser_impl_global_constant_decl_test.cc b/src/tint/reader/wgsl/parser_impl_global_constant_decl_test.cc index bfa678e3ed..46bfa34528 100644 --- a/src/tint/reader/wgsl/parser_impl_global_constant_decl_test.cc +++ b/src/tint/reader/wgsl/parser_impl_global_constant_decl_test.cc @@ -204,6 +204,36 @@ TEST_F(ParserImplTest, GlobalOverrideDecl_WithId) { EXPECT_EQ(override_attr->value, 7u); } +TEST_F(ParserImplTest, GlobalOverrideDecl_WithId_TrailingComma) { + auto p = parser("@id(7,) override a : f32 = 1."); + auto attrs = p->attribute_list(); + EXPECT_FALSE(attrs.errored); + EXPECT_TRUE(attrs.matched); + + auto e = p->global_constant_decl(attrs.value); + EXPECT_FALSE(p->has_error()) << p->error(); + EXPECT_TRUE(e.matched); + EXPECT_FALSE(e.errored); + auto* override = e.value->As(); + ASSERT_NE(override, nullptr); + + EXPECT_EQ(override->symbol, p->builder().Symbols().Get("a")); + ASSERT_NE(override->type, nullptr); + EXPECT_TRUE(override->type->Is()); + + EXPECT_EQ(override->source.range.begin.line, 1u); + EXPECT_EQ(override->source.range.begin.column, 18u); + EXPECT_EQ(override->source.range.end.line, 1u); + EXPECT_EQ(override->source.range.end.column, 19u); + + ASSERT_NE(override->constructor, nullptr); + EXPECT_TRUE(override->constructor->Is()); + + auto* override_attr = ast::GetAttribute(override->attributes); + ASSERT_NE(override_attr, nullptr); + EXPECT_EQ(override_attr->value, 7u); +} + TEST_F(ParserImplTest, GlobalOverrideDecl_WithoutId) { auto p = parser("override a : f32 = 1."); auto attrs = p->attribute_list(); 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 d185f3736a..df75f96984 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 @@ -33,6 +33,22 @@ TEST_F(ParserImplTest, Attribute_Size) { EXPECT_EQ(o->size, 4u); } +TEST_F(ParserImplTest, Attribute_Size_TrailingComma) { + auto p = parser("size(4,)"); + auto attr = p->attribute(); + EXPECT_TRUE(attr.matched); + EXPECT_FALSE(attr.errored); + ASSERT_NE(attr.value, nullptr); + ASSERT_FALSE(p->has_error()); + + auto* member_attr = attr.value->As(); + ASSERT_NE(member_attr, nullptr); + ASSERT_TRUE(member_attr->Is()); + + auto* o = member_attr->As(); + EXPECT_EQ(o->size, 4u); +} + TEST_F(ParserImplTest, Attribute_Size_MissingLeftParen) { auto p = parser("size 4)"); auto attr = p->attribute(); @@ -89,6 +105,22 @@ TEST_F(ParserImplTest, Attribute_Align) { EXPECT_EQ(o->align, 4u); } +TEST_F(ParserImplTest, Attribute_Align_TrailingComma) { + auto p = parser("align(4,)"); + auto attr = p->attribute(); + EXPECT_TRUE(attr.matched); + EXPECT_FALSE(attr.errored); + ASSERT_NE(attr.value, nullptr); + ASSERT_FALSE(p->has_error()); + + auto* member_attr = attr.value->As(); + ASSERT_NE(member_attr, nullptr); + ASSERT_TRUE(member_attr->Is()); + + auto* o = member_attr->As(); + EXPECT_EQ(o->align, 4u); +} + TEST_F(ParserImplTest, Attribute_Align_MissingLeftParen) { auto p = parser("align 4)"); auto attr = p->attribute(); diff --git a/src/tint/reader/wgsl/parser_impl_variable_attribute_test.cc b/src/tint/reader/wgsl/parser_impl_variable_attribute_test.cc index 8833273f27..01e4260322 100644 --- a/src/tint/reader/wgsl/parser_impl_variable_attribute_test.cc +++ b/src/tint/reader/wgsl/parser_impl_variable_attribute_test.cc @@ -32,6 +32,21 @@ TEST_F(ParserImplTest, Attribute_Location) { EXPECT_EQ(loc->value, 4u); } +TEST_F(ParserImplTest, Attribute_Location_TrailingComma) { + auto p = parser("location(4,)"); + auto attr = p->attribute(); + EXPECT_TRUE(attr.matched); + EXPECT_FALSE(attr.errored); + ASSERT_NE(attr.value, nullptr); + auto* var_attr = attr.value->As(); + ASSERT_NE(var_attr, nullptr); + ASSERT_FALSE(p->has_error()); + ASSERT_TRUE(var_attr->Is()); + + auto* loc = var_attr->As(); + EXPECT_EQ(loc->value, 4u); +} + TEST_F(ParserImplTest, Attribute_Location_MissingLeftParen) { auto p = parser("location 4)"); auto attr = p->attribute(); @@ -99,6 +114,22 @@ TEST_P(BuiltinTest, Attribute_Builtin) { auto* builtin = var_attr->As(); EXPECT_EQ(builtin->builtin, params.result); } +TEST_P(BuiltinTest, Attribute_Builtin_TrailingComma) { + auto params = GetParam(); + auto p = parser(std::string("builtin(") + params.input + ",)"); + + auto attr = p->attribute(); + EXPECT_TRUE(attr.matched); + EXPECT_FALSE(attr.errored); + ASSERT_NE(attr.value, nullptr); + auto* var_attr = attr.value->As(); + ASSERT_FALSE(p->has_error()) << p->error(); + ASSERT_NE(var_attr, nullptr); + ASSERT_TRUE(var_attr->Is()); + + auto* builtin = var_attr->As(); + EXPECT_EQ(builtin->builtin, params.result); +} INSTANTIATE_TEST_SUITE_P( ParserImplTest, BuiltinTest, @@ -182,6 +213,32 @@ TEST_F(ParserImplTest, Attribute_Interpolate_Flat) { EXPECT_EQ(interp->sampling, ast::InterpolationSampling::kNone); } +TEST_F(ParserImplTest, Attribute_Interpolate_Single_TrailingComma) { + auto p = parser("interpolate(flat,)"); + auto attr = p->attribute(); + EXPECT_TRUE(attr.matched); + EXPECT_FALSE(attr.errored); + ASSERT_NE(attr.value, nullptr); + auto* var_attr = attr.value->As(); + ASSERT_NE(var_attr, nullptr); + ASSERT_FALSE(p->has_error()); + ASSERT_TRUE(var_attr->Is()); + + auto* interp = var_attr->As(); + EXPECT_EQ(interp->type, ast::InterpolationType::kFlat); + EXPECT_EQ(interp->sampling, ast::InterpolationSampling::kNone); +} + +TEST_F(ParserImplTest, Attribute_Interpolate_Single_DoubleTrailingComma) { + auto p = parser("interpolate(flat,,)"); + auto attr = p->attribute(); + EXPECT_FALSE(attr.matched); + EXPECT_TRUE(attr.errored); + EXPECT_EQ(attr.value, nullptr); + EXPECT_TRUE(p->has_error()); + EXPECT_EQ(p->error(), "1:18: invalid interpolation sampling"); +} + TEST_F(ParserImplTest, Attribute_Interpolate_Perspective_Center) { auto p = parser("interpolate(perspective, center)"); auto attr = p->attribute(); @@ -198,6 +255,22 @@ TEST_F(ParserImplTest, Attribute_Interpolate_Perspective_Center) { EXPECT_EQ(interp->sampling, ast::InterpolationSampling::kCenter); } +TEST_F(ParserImplTest, Attribute_Interpolate_Double_TrailingComma) { + auto p = parser("interpolate(perspective, center,)"); + auto attr = p->attribute(); + EXPECT_TRUE(attr.matched); + EXPECT_FALSE(attr.errored); + ASSERT_NE(attr.value, nullptr); + auto* var_attr = attr.value->As(); + ASSERT_NE(var_attr, nullptr); + ASSERT_FALSE(p->has_error()); + ASSERT_TRUE(var_attr->Is()); + + auto* interp = var_attr->As(); + EXPECT_EQ(interp->type, ast::InterpolationType::kPerspective); + EXPECT_EQ(interp->sampling, ast::InterpolationSampling::kCenter); +} + TEST_F(ParserImplTest, Attribute_Interpolate_Perspective_Centroid) { auto p = parser("interpolate(perspective, centroid)"); auto attr = p->attribute(); @@ -270,16 +343,6 @@ TEST_F(ParserImplTest, Attribute_Interpolate_InvalidFirstValue) { EXPECT_EQ(p->error(), "1:13: invalid interpolation type"); } -TEST_F(ParserImplTest, Attribute_Interpolate_MissingSecondValue) { - auto p = parser("interpolate(perspective,)"); - auto attr = p->attribute(); - EXPECT_FALSE(attr.matched); - EXPECT_TRUE(attr.errored); - EXPECT_EQ(attr.value, nullptr); - EXPECT_TRUE(p->has_error()); - EXPECT_EQ(p->error(), "1:25: invalid interpolation sampling"); -} - TEST_F(ParserImplTest, Attribute_Interpolate_InvalidSecondValue) { auto p = parser("interpolate(perspective, nope)"); auto attr = p->attribute(); @@ -305,6 +368,21 @@ TEST_F(ParserImplTest, Attribute_Binding) { EXPECT_EQ(binding->value, 4u); } +TEST_F(ParserImplTest, Attribute_Binding_TrailingComma) { + auto p = parser("binding(4,)"); + auto attr = p->attribute(); + EXPECT_TRUE(attr.matched); + EXPECT_FALSE(attr.errored); + ASSERT_NE(attr.value, nullptr); + auto* var_attr = attr.value->As(); + ASSERT_NE(var_attr, nullptr); + ASSERT_FALSE(p->has_error()); + ASSERT_TRUE(var_attr->Is()); + + auto* binding = var_attr->As(); + EXPECT_EQ(binding->value, 4u); +} + TEST_F(ParserImplTest, Attribute_Binding_MissingLeftParen) { auto p = parser("binding 4)"); auto attr = p->attribute(); @@ -360,6 +438,21 @@ TEST_F(ParserImplTest, Attribute_group) { EXPECT_EQ(group->value, 4u); } +TEST_F(ParserImplTest, Attribute_group_TrailingComma) { + auto p = parser("group(4,)"); + auto attr = p->attribute(); + EXPECT_TRUE(attr.matched); + EXPECT_FALSE(attr.errored); + ASSERT_NE(attr.value, nullptr); + auto* var_attr = attr.value->As(); + ASSERT_FALSE(p->has_error()); + ASSERT_NE(var_attr, nullptr); + ASSERT_TRUE(var_attr->Is()); + + auto* group = var_attr->As(); + EXPECT_EQ(group->value, 4u); +} + TEST_F(ParserImplTest, Attribute_Group_MissingLeftParen) { auto p = parser("group 2)"); auto attr = p->attribute();