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 <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>

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
 

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",

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

src/tint/reader/wgsl/parser_impl.cc

@@ -1613,29 +1613,6 @@ Expect<ast::Parameter*> 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<ast::PipelineStage> 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<const ast::Attribute*> 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<ast::StageAttribute>(t.source(), stage.value);
-        });
-    }
-
     if (t == "vertex") {
         return create<ast::StageAttribute>(t.source(), ast::PipelineStage::kVertex);
     }

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

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<ast::Attribute>();
-    auto* attr_1 = attrs.value[1]->As<ast::Attribute>();
-    ASSERT_NE(attr_0, nullptr);
-    ASSERT_NE(attr_1, nullptr);
-
-    ASSERT_TRUE(attr_0->Is<ast::WorkgroupAttribute>());
-    const ast::Expression* x = attr_0->As<ast::WorkgroupAttribute>()->x;
-    ASSERT_NE(x, nullptr);
-    auto* x_literal = x->As<ast::LiteralExpression>();
-    ASSERT_NE(x_literal, nullptr);
-    ASSERT_TRUE(x_literal->Is<ast::IntLiteralExpression>());
-    EXPECT_EQ(x_literal->As<ast::IntLiteralExpression>()->value, 2);
-    EXPECT_EQ(x_literal->As<ast::IntLiteralExpression>()->suffix,
-              ast::IntLiteralExpression::Suffix::kNone);
-    ASSERT_TRUE(attr_1->Is<ast::StageAttribute>());
-    EXPECT_EQ(attr_1->As<ast::StageAttribute>()->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

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<ast::Attribute>();
-    ASSERT_NE(func_attr, nullptr);
-    ASSERT_TRUE(func_attr->Is<ast::StageAttribute>());
-    EXPECT_EQ(func_attr->As<ast::StageAttribute>()->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();

src/tint/reader/wgsl/parser_impl_pipeline_stage_test.cc

@@ -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<PipelineStageData> {};
-
-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