From f218af04610a66598d44ecbba6c2b17bb1e78433 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Mon, 7 Nov 2022 11:34:34 +0000 Subject: [PATCH] Remove the `@stage` attribute This CL removes the `@stage` attribute. Bug: tint:1503 Change-Id: I1e31e96056a053f0c1e92b4bfc3c63c3592e9765 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/108860 Reviewed-by: Ben Clayton Commit-Queue: Ben Clayton Kokoro: Kokoro --- docs/tint/origin-trial-changes.md | 2 + src/tint/BUILD.gn | 3 +- src/tint/CMakeLists.txt | 1 - src/tint/reader/wgsl/parser_impl.cc | 51 --------------- .../reader/wgsl/parser_impl_error_msg_test.cc | 42 ------------- ...arser_impl_function_attribute_list_test.cc | 36 ----------- .../parser_impl_function_attribute_test.cc | 55 ---------------- .../wgsl/parser_impl_pipeline_stage_test.cc | 62 ------------------- 8 files changed, 3 insertions(+), 249 deletions(-) delete mode 100644 src/tint/reader/wgsl/parser_impl_pipeline_stage_test.cc diff --git a/docs/tint/origin-trial-changes.md b/docs/tint/origin-trial-changes.md index a509f8358c..5d36a132e3 100644 --- a/docs/tint/origin-trial-changes.md +++ b/docs/tint/origin-trial-changes.md @@ -5,6 +5,8 @@ ### Breaking changes * `textureDimensions()`, `textureNumLayers()` and `textureNumLevels()` now return unsigned integers / vectors. [tint:1526](crbug.com/tint/1526) +* The `@stage` attribute has been removed. The short forms should be used + instead (`@vertex`, `@fragment`, or `@compute`). [tint:1503](crbug.com/tint/1503) ### New features diff --git a/src/tint/BUILD.gn b/src/tint/BUILD.gn index 337b6a1ff3..39fae600e2 100644 --- a/src/tint/BUILD.gn +++ b/src/tint/BUILD.gn @@ -581,8 +581,8 @@ libtint_source_set("libtint_core_all_src") { "utils/enum_set.h", "utils/foreach_macro.h", "utils/hash.h", - "utils/hashmap_base.h", "utils/hashmap.h", + "utils/hashmap_base.h", "utils/hashset.h", "utils/map.h", "utils/math.h", @@ -1423,7 +1423,6 @@ if (tint_build_unittests) { "reader/wgsl/parser_impl_multiplicative_expression_test.cc", "reader/wgsl/parser_impl_param_list_test.cc", "reader/wgsl/parser_impl_paren_expression_test.cc", - "reader/wgsl/parser_impl_pipeline_stage_test.cc", "reader/wgsl/parser_impl_primary_expression_test.cc", "reader/wgsl/parser_impl_relational_expression_test.cc", "reader/wgsl/parser_impl_reserved_keyword_test.cc", diff --git a/src/tint/CMakeLists.txt b/src/tint/CMakeLists.txt index a22d9fcfe4..ca4456f41c 100644 --- a/src/tint/CMakeLists.txt +++ b/src/tint/CMakeLists.txt @@ -1054,7 +1054,6 @@ if(TINT_BUILD_TESTS) reader/wgsl/parser_impl_multiplicative_expression_test.cc reader/wgsl/parser_impl_param_list_test.cc reader/wgsl/parser_impl_paren_expression_test.cc - reader/wgsl/parser_impl_pipeline_stage_test.cc reader/wgsl/parser_impl_primary_expression_test.cc reader/wgsl/parser_impl_relational_expression_test.cc reader/wgsl/parser_impl_reserved_keyword_test.cc diff --git a/src/tint/reader/wgsl/parser_impl.cc b/src/tint/reader/wgsl/parser_impl.cc index 976706a115..dc02ca4d53 100644 --- a/src/tint/reader/wgsl/parser_impl.cc +++ b/src/tint/reader/wgsl/parser_impl.cc @@ -1613,29 +1613,6 @@ Expect ParserImpl::expect_param() { std::move(attrs.value)); // attributes } -// pipeline_stage -// : VERTEX -// | FRAGMENT -// | COMPUTE -// -// TODO(crbug.com/tint/1503): Remove when deprecation period is over. -Expect ParserImpl::expect_pipeline_stage() { - auto& t = peek(); - if (t == "vertex") { - next(); // Consume the peek - return {ast::PipelineStage::kVertex, t.source()}; - } - if (t == "fragment") { - next(); // Consume the peek - return {ast::PipelineStage::kFragment, t.source()}; - } - if (t == "compute") { - next(); // Consume the peek - return {ast::PipelineStage::kCompute, t.source()}; - } - return add_error(peek(), "invalid value for stage attribute"); -} - // interpolation_sample_name // : 'center' // | 'centroid' @@ -3637,34 +3614,6 @@ Maybe ParserImpl::attribute() { }); } - // TODO(crbug.com/tint/1503): Remove when deprecation period is over. - if (t == "stage") { - return expect_paren_block("stage attribute", [&]() -> Result { - auto stage = expect_pipeline_stage(); - if (stage.errored) { - return Failure::kErrored; - } - - std::string warning = "remove stage and use @"; - switch (stage.value) { - case ast::PipelineStage::kVertex: - warning += "vertex"; - break; - case ast::PipelineStage::kFragment: - warning += "fragment"; - break; - case ast::PipelineStage::kCompute: - warning += "compute"; - break; - case ast::PipelineStage::kNone: - break; - } - deprecated(t.source(), warning); - - return create(t.source(), stage.value); - }); - } - if (t == "vertex") { return create(t.source(), ast::PipelineStage::kVertex); } 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 fbfe6f673b..8b8a634bba 100644 --- a/src/tint/reader/wgsl/parser_impl_error_msg_test.cc +++ b/src/tint/reader/wgsl/parser_impl_error_msg_test.cc @@ -351,48 +351,6 @@ fn f() { static_assert true } )"); } -// TODO(crbug.com/tint/1503): Remove this when @stage is removed -TEST_F(ParserImplErrorTest, FunctionDeclStageMissingLParen) { - EXPECT("@stage vertex) fn f() {}", - R"(test.wgsl:1:8 error: expected '(' for stage attribute -@stage vertex) fn f() {} - ^^^^^^ -)"); -} - -TEST_F(ParserImplErrorTest, FunctionDeclStageMissingRParen) { - EXPECT( - "@stage(vertex fn f() {}", - R"(test.wgsl:1:2 warning: use of deprecated language feature: remove stage and use @vertex -@stage(vertex fn f() {} - ^^^^^ - -test.wgsl:1:15 error: expected ')' for stage attribute -@stage(vertex fn f() {} - ^^ -)"); -} - -TEST_F(ParserImplErrorTest, FunctionDeclStageInvalid) { - EXPECT("@stage(x) fn f() {}", - R"(test.wgsl:1:8 error: invalid value for stage attribute -@stage(x) fn f() {} - ^ -)"); -} - -TEST_F(ParserImplErrorTest, FunctionDeclStageTypeInvalid) { - EXPECT("@shader(vertex) fn main() {}", - R"(test.wgsl:1:2 error: expected attribute -@shader(vertex) fn main() {} - ^^^^^^ - -test.wgsl:1:8 error: unexpected token -@shader(vertex) fn main() {} - ^ -)"); -} - TEST_F(ParserImplErrorTest, FunctionDeclWorkgroupSizeXInvalid) { EXPECT("@workgroup_size() fn f() {}", R"(test.wgsl:1:17 error: expected workgroup_size x parameter diff --git a/src/tint/reader/wgsl/parser_impl_function_attribute_list_test.cc b/src/tint/reader/wgsl/parser_impl_function_attribute_list_test.cc index 28b84a9767..785d23c4a4 100644 --- a/src/tint/reader/wgsl/parser_impl_function_attribute_list_test.cc +++ b/src/tint/reader/wgsl/parser_impl_function_attribute_list_test.cc @@ -18,33 +18,6 @@ namespace tint::reader::wgsl { namespace { -// TODO(crbug.com/tint/1503): Remove this when @stage is removed -TEST_F(ParserImplTest, AttributeList_Parses_Stage) { - auto p = parser("@workgroup_size(2) @stage(compute)"); - auto attrs = p->attribute_list(); - EXPECT_FALSE(p->has_error()) << p->error(); - EXPECT_FALSE(attrs.errored); - EXPECT_TRUE(attrs.matched); - ASSERT_EQ(attrs.value.Length(), 2u); - - auto* attr_0 = attrs.value[0]->As(); - auto* attr_1 = attrs.value[1]->As(); - ASSERT_NE(attr_0, nullptr); - ASSERT_NE(attr_1, nullptr); - - ASSERT_TRUE(attr_0->Is()); - const ast::Expression* x = attr_0->As()->x; - ASSERT_NE(x, nullptr); - auto* x_literal = x->As(); - ASSERT_NE(x_literal, nullptr); - ASSERT_TRUE(x_literal->Is()); - EXPECT_EQ(x_literal->As()->value, 2); - EXPECT_EQ(x_literal->As()->suffix, - ast::IntLiteralExpression::Suffix::kNone); - ASSERT_TRUE(attr_1->Is()); - EXPECT_EQ(attr_1->As()->stage, ast::PipelineStage::kCompute); -} - TEST_F(ParserImplTest, AttributeList_Parses) { auto p = parser("@workgroup_size(2) @compute"); auto attrs = p->attribute_list(); @@ -81,14 +54,5 @@ TEST_F(ParserImplTest, AttributeList_Invalid) { EXPECT_EQ(p->error(), "1:2: expected attribute"); } -TEST_F(ParserImplTest, AttributeList_BadAttribute) { - auto p = parser("@stage()"); - auto attrs = p->attribute_list(); - EXPECT_TRUE(p->has_error()); - EXPECT_TRUE(attrs.errored); - EXPECT_FALSE(attrs.matched); - EXPECT_EQ(p->error(), "1:8: invalid value for stage attribute"); -} - } // namespace } // namespace tint::reader::wgsl 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 60e8f861c8..186c86a21b 100644 --- a/src/tint/reader/wgsl/parser_impl_function_attribute_test.cc +++ b/src/tint/reader/wgsl/parser_impl_function_attribute_test.cc @@ -397,61 +397,6 @@ TEST_F(ParserImplTest, Attribute_Workgroup_Missing_Z_Comma) { EXPECT_EQ(p->error(), "1:21: expected ')' for workgroup_size attribute"); } -// TODO(crbug.com/tint/1503): Remove when @stage is removed -TEST_F(ParserImplTest, Attribute_Stage) { - auto p = parser("stage(compute)"); - 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()); - EXPECT_EQ(func_attr->As()->stage, ast::PipelineStage::kCompute); -} - -TEST_F(ParserImplTest, Attribute_Stage_MissingValue) { - auto p = parser("stage()"); - 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:7: invalid value for stage attribute"); -} - -TEST_F(ParserImplTest, Attribute_Stage_MissingInvalid) { - auto p = parser("stage(nan)"); - 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:7: invalid value for stage attribute"); -} - -TEST_F(ParserImplTest, Attribute_Stage_MissingLeftParen) { - auto p = parser("stage compute)"); - 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:7: expected '(' for stage attribute"); -} - -TEST_F(ParserImplTest, Attribute_Stage_MissingRightParen) { - auto p = parser("stage(compute"); - 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(), R"(1:1: use of deprecated language feature: remove stage and use @compute -1:14: expected ')' for stage attribute)"); -} - TEST_F(ParserImplTest, Attribute_Compute) { auto p = parser("compute"); auto attr = p->attribute(); diff --git a/src/tint/reader/wgsl/parser_impl_pipeline_stage_test.cc b/src/tint/reader/wgsl/parser_impl_pipeline_stage_test.cc deleted file mode 100644 index 3c730e2ab1..0000000000 --- a/src/tint/reader/wgsl/parser_impl_pipeline_stage_test.cc +++ /dev/null @@ -1,62 +0,0 @@ -// Copyright 2020 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 { - -struct PipelineStageData { - std::string input; - ast::PipelineStage result; -}; -inline std::ostream& operator<<(std::ostream& out, PipelineStageData data) { - return out << data.input; -} - -class PipelineStageTest : public ParserImplTestWithParam {}; - -TEST_P(PipelineStageTest, Parses) { - auto params = GetParam(); - auto p = parser(params.input); - - auto stage = p->expect_pipeline_stage(); - ASSERT_FALSE(p->has_error()) << p->error(); - ASSERT_FALSE(stage.errored); - EXPECT_EQ(stage.value, params.result); - EXPECT_EQ(stage.source.range.begin.line, 1u); - EXPECT_EQ(stage.source.range.begin.column, 1u); - EXPECT_EQ(stage.source.range.end.line, 1u); - EXPECT_EQ(stage.source.range.end.column, 1u + params.input.size()); - - auto& t = p->next(); - EXPECT_TRUE(t.IsEof()); -} -INSTANTIATE_TEST_SUITE_P( - ParserImplTest, - PipelineStageTest, - testing::Values(PipelineStageData{"vertex", ast::PipelineStage::kVertex}, - PipelineStageData{"fragment", ast::PipelineStage::kFragment}, - PipelineStageData{"compute", ast::PipelineStage::kCompute})); - -TEST_F(ParserImplTest, PipelineStage_NoMatch) { - auto p = parser("not-a-stage"); - auto stage = p->expect_pipeline_stage(); - ASSERT_TRUE(p->has_error()); - ASSERT_TRUE(stage.errored); - ASSERT_EQ(p->error(), "1:1: invalid value for stage attribute"); -} - -} // namespace -} // namespace tint::reader::wgsl