Add `SuggestAlternatives` to attributes.

This CL adds an `attribute` enum into intrinsics.def and updates the
WGSL parser to use the enum parser and SuggestAlternatives.

Bug: tint:1831
Change-Id: I33b3e6bbf092282d9b1f86a1080e69940f55ff68
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/121280
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Dan Sinclair <dsinclair@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>

src/tint/BUILD.gn

@@ -701,6 +701,8 @@ libtint_source_set("libtint_builtins_src") {
     "builtin/access.h",
     "builtin/address_space.cc",
     "builtin/address_space.h",
+    "builtin/attribute.cc",
+    "builtin/attribute.h",
     "builtin/builtin.cc",
     "builtin/builtin.h",
     "builtin/builtin_value.cc",
@@ -1311,6 +1313,7 @@ if (tint_build_unittests) {
     sources = [
       "builtin/access_test.cc",
       "builtin/address_space_test.cc",
+      "builtin/attribute_test.cc",
       "builtin/builtin_test.cc",
       "builtin/builtin_value_test.cc",
       "builtin/diagnostic_rule_test.cc",

src/tint/CMakeLists.txt

@@ -555,6 +555,7 @@ list(APPEND TINT_LIB_SRCS
 
 tint_generated(builtin/access BENCH TEST)
 tint_generated(builtin/address_space BENCH TEST)
+tint_generated(builtin/attribute BENCH TEST)
 tint_generated(builtin/builtin BENCH TEST)
 tint_generated(builtin/builtin_value BENCH TEST)
 tint_generated(builtin/diagnostic_rule BENCH TEST)

src/tint/builtin/attribute.cc

@@ -0,0 +1,117 @@
+// Copyright 2023 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.
+
+////////////////////////////////////////////////////////////////////////////////
+// File generated by tools/src/cmd/gen
+// using the template:
+//   src/tint/builtin/attribute.cc.tmpl
+//
+// Do not modify this file directly
+////////////////////////////////////////////////////////////////////////////////
+
+#include "src/tint/builtin/attribute.h"
+
+namespace tint::builtin {
+
+/// ParseAttribute parses a Attribute from a string.
+/// @param str the string to parse
+/// @returns the parsed enum, or Attribute::kUndefined if the string could not be parsed.
+Attribute ParseAttribute(std::string_view str) {
+    if (str == "align") {
+        return Attribute::kAlign;
+    }
+    if (str == "binding") {
+        return Attribute::kBinding;
+    }
+    if (str == "builtin") {
+        return Attribute::kBuiltin;
+    }
+    if (str == "compute") {
+        return Attribute::kCompute;
+    }
+    if (str == "diagnostic") {
+        return Attribute::kDiagnostic;
+    }
+    if (str == "fragment") {
+        return Attribute::kFragment;
+    }
+    if (str == "group") {
+        return Attribute::kGroup;
+    }
+    if (str == "id") {
+        return Attribute::kId;
+    }
+    if (str == "interpolate") {
+        return Attribute::kInterpolate;
+    }
+    if (str == "invariant") {
+        return Attribute::kInvariant;
+    }
+    if (str == "location") {
+        return Attribute::kLocation;
+    }
+    if (str == "must_use") {
+        return Attribute::kMustUse;
+    }
+    if (str == "size") {
+        return Attribute::kSize;
+    }
+    if (str == "vertex") {
+        return Attribute::kVertex;
+    }
+    if (str == "workgroup_size") {
+        return Attribute::kWorkgroupSize;
+    }
+    return Attribute::kUndefined;
+}
+
+std::ostream& operator<<(std::ostream& out, Attribute value) {
+    switch (value) {
+        case Attribute::kUndefined:
+            return out << "undefined";
+        case Attribute::kAlign:
+            return out << "align";
+        case Attribute::kBinding:
+            return out << "binding";
+        case Attribute::kBuiltin:
+            return out << "builtin";
+        case Attribute::kCompute:
+            return out << "compute";
+        case Attribute::kDiagnostic:
+            return out << "diagnostic";
+        case Attribute::kFragment:
+            return out << "fragment";
+        case Attribute::kGroup:
+            return out << "group";
+        case Attribute::kId:
+            return out << "id";
+        case Attribute::kInterpolate:
+            return out << "interpolate";
+        case Attribute::kInvariant:
+            return out << "invariant";
+        case Attribute::kLocation:
+            return out << "location";
+        case Attribute::kMustUse:
+            return out << "must_use";
+        case Attribute::kSize:
+            return out << "size";
+        case Attribute::kVertex:
+            return out << "vertex";
+        case Attribute::kWorkgroupSize:
+            return out << "workgroup_size";
+    }
+    return out << "<unknown>";
+}
+
+}  // namespace tint::builtin

src/tint/builtin/attribute.cc.tmpl

@@ -0,0 +1,25 @@
+{{- /*
+--------------------------------------------------------------------------------
+Template file for use with tools/src/cmd/gen to generate attribute.cc
+
+To update the generated file, run:
+    ./tools/run gen
+
+See:
+* tools/src/cmd/gen for structures used by this template
+* https://golang.org/pkg/text/template/ for documentation on the template syntax
+--------------------------------------------------------------------------------
+*/ -}}
+
+{{- Import "src/tint/templates/enums.tmpl.inc" -}}
+{{- $enum := (Sem.Enum "attribute") -}}
+
+#include "src/tint/builtin/attribute.h"
+
+namespace tint::builtin {
+
+{{ Eval "ParseEnum" $enum}}
+
+{{ Eval "EnumOStream" $enum}}
+
+}  // namespace tint::builtin

src/tint/builtin/attribute.h

@@ -0,0 +1,72 @@
+// Copyright 2023 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.
+
+////////////////////////////////////////////////////////////////////////////////
+// File generated by tools/src/cmd/gen
+// using the template:
+//   src/tint/builtin/attribute.h.tmpl
+//
+// Do not modify this file directly
+////////////////////////////////////////////////////////////////////////////////
+
+#ifndef SRC_TINT_BUILTIN_ATTRIBUTE_H_
+#define SRC_TINT_BUILTIN_ATTRIBUTE_H_
+
+#include <ostream>
+
+/// \cond DO_NOT_DOCUMENT
+/// There is a bug in doxygen where this enum conflicts with the ast::Attribute
+/// and generates invalid documentation errors.
+namespace tint::builtin {
+
+/// Address space of a given pointer.
+enum class Attribute {
+    kUndefined,
+    kAlign,
+    kBinding,
+    kBuiltin,
+    kCompute,
+    kDiagnostic,
+    kFragment,
+    kGroup,
+    kId,
+    kInterpolate,
+    kInvariant,
+    kLocation,
+    kMustUse,
+    kSize,
+    kVertex,
+    kWorkgroupSize,
+};
+
+/// @param out the std::ostream to write to
+/// @param value the Attribute
+/// @returns `out` so calls can be chained
+std::ostream& operator<<(std::ostream& out, Attribute value);
+
+/// ParseAttribute parses a Attribute from a string.
+/// @param str the string to parse
+/// @returns the parsed enum, or Attribute::kUndefined if the string could not be parsed.
+Attribute ParseAttribute(std::string_view str);
+
+constexpr const char* kAttributeStrings[] = {
+    "align",    "binding",  "builtin", "compute",     "diagnostic",
+    "fragment", "group",    "id",      "interpolate", "invariant",
+    "location", "must_use", "size",    "vertex",      "workgroup_size",
+};
+
+}  // namespace tint::builtin
+/// \endcond
+
+#endif  // SRC_TINT_BUILTIN_ATTRIBUTE_H_

src/tint/builtin/attribute.h.tmpl

@@ -0,0 +1,33 @@
+{{- /*
+--------------------------------------------------------------------------------
+Template file for use with tools/src/cmd/gen to generate attribute.h
+
+To update the generated file, run:
+    ./tools/run gen
+
+See:
+* tools/src/cmd/gen for structures used by this template
+* https://golang.org/pkg/text/template/ for documentation on the template syntax
+--------------------------------------------------------------------------------
+*/ -}}
+
+{{- Import "src/tint/templates/enums.tmpl.inc" -}}
+{{- $enum := (Sem.Enum "attribute") -}}
+
+#ifndef SRC_TINT_BUILTIN_ATTRIBUTE_H_
+#define SRC_TINT_BUILTIN_ATTRIBUTE_H_
+
+#include <ostream>
+
+/// \cond DO_NOT_DOCUMENT
+/// There is a bug in doxygen where this enum conflicts with the ast::Attribute
+/// and generates invalid documentation errors.
+namespace tint::builtin {
+
+/// Address space of a given pointer.
+{{ Eval "DeclareEnum" $enum}}
+
+}  // namespace tint::builtin
+/// \endcond
+
+#endif  // SRC_TINT_BUILTIN_ATTRIBUTE_H_

src/tint/builtin/attribute_bench.cc

@@ -0,0 +1,151 @@
+// Copyright 2023 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.
+
+////////////////////////////////////////////////////////////////////////////////
+// File generated by tools/src/cmd/gen
+// using the template:
+//   src/tint/builtin/attribute_bench.cc.tmpl
+//
+// Do not modify this file directly
+////////////////////////////////////////////////////////////////////////////////
+
+#include "src/tint/builtin/attribute.h"
+
+#include <array>
+
+#include "benchmark/benchmark.h"
+
+namespace tint::builtin {
+namespace {
+
+void AttributeParser(::benchmark::State& state) {
+    const char* kStrings[] = {
+        "alccn",
+        "3g",
+        "aVign",
+        "align",
+        "alig1",
+        "qqlJn",
+        "alill7n",
+        "ppqqndnHH",
+        "bicv",
+        "bndiGg",
+        "binding",
+        "bindiivg",
+        "8WWnding",
+        "bxxding",
+        "bXltigg",
+        "ultXn",
+        "buil3in",
+        "builtin",
+        "Euiltin",
+        "bPTTltin",
+        "builtdxx",
+        "c44mpute",
+        "coSSpuVVe",
+        "RomR22e",
+        "compute",
+        "cFpu9e",
+        "comute",
+        "VOORRHte",
+        "dyagnstic",
+        "d77agnnnsllrrc",
+        "dia400ostic",
+        "diagnostic",
+        "danstooc",
+        "dignszzic",
+        "d11ansppiic",
+        "XXragment",
+        "fIIa9955nnnt",
+        "aarHHgmenYSS",
+        "fragment",
+        "fkkaet",
+        "gjamRRn",
+        "fabmnt",
+        "gjoup",
+        "goup",
+        "goq",
+        "group",
+        "Nroup",
+        "govv",
+        "gruQQ",
+        "r",
+        "jd",
+        "NNw",
+        "id",
+        "i",
+        "rrd",
+        "iG",
+        "FFnterpolate",
+        "iEtrplat",
+        "inerporrate",
+        "interpolate",
+        "inteplate",
+        "XterJJoDate",
+        "inepol8t",
+        "nvark1n",
+        "invriant",
+        "Jnvarant",
+        "invariant",
+        "invaricnt",
+        "invariaOt",
+        "invttKK_ianvv",
+        "lxxcati8",
+        "Focqq__o",
+        "locaiqqn",
+        "location",
+        "loc33tio6",
+        "ltto6at9QQn",
+        "loc66tio",
+        "mOxt_u6zz",
+        "musyy_use",
+        "mHH_use",
+        "must_use",
+        "qWW4st_se",
+        "musOO_se",
+        "ust_uYe",
+        "i",
+        "Fie",
+        "siw",
+        "size",
+        "zff",
+        "sizqK",
+        "s3zmm",
+        "ertex",
+        "vereq",
+        "vbtbbx",
+        "vertex",
+        "irtex",
+        "vOOteq",
+        "vertTvvx",
+        "woFFkgroup_size",
+        "wfr00grPupsiQe",
+        "workgrouP_size",
+        "workgroup_size",
+        "workgroup77sise",
+        "RRobbkgroupCsize",
+        "wXXrkgroup_size",
+    };
+    for (auto _ : state) {
+        for (auto* str : kStrings) {
+            auto result = ParseAttribute(str);
+            benchmark::DoNotOptimize(result);
+        }
+    }
+}
+
+BENCHMARK(AttributeParser);
+
+}  // namespace
+}  // namespace tint::builtin

src/tint/builtin/attribute_bench.cc.tmpl

@@ -0,0 +1,29 @@
+{{- /*
+--------------------------------------------------------------------------------
+Template file for use with tools/src/cmd/gen to generate attribute_bench.cc
+
+To update the generated file, run:
+    ./tools/run gen
+
+See:
+* tools/src/cmd/gen for structures used by this template
+* https://golang.org/pkg/text/template/ for documentation on the template syntax
+--------------------------------------------------------------------------------
+*/ -}}
+
+{{- Import "src/tint/templates/enums.tmpl.inc" -}}
+{{- $enum := (Sem.Enum "attribute") -}}
+
+#include "src/tint/builtin/attribute.h"
+
+#include <array>
+
+#include "benchmark/benchmark.h"
+
+namespace tint::builtin {
+namespace {
+
+{{ Eval "BenchmarkParseEnum" $enum }}
+
+}  // namespace
+}  // namespace tint::builtin

src/tint/builtin/attribute_test.cc

@@ -0,0 +1,135 @@
+// Copyright 2023 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.
+
+////////////////////////////////////////////////////////////////////////////////
+// File generated by tools/src/cmd/gen
+// using the template:
+//   src/tint/builtin/attribute_test.cc.tmpl
+//
+// Do not modify this file directly
+////////////////////////////////////////////////////////////////////////////////
+
+#include "src/tint/builtin/attribute.h"
+
+#include <gtest/gtest.h>
+
+#include <string>
+
+#include "src/tint/utils/string.h"
+
+namespace tint::builtin {
+namespace {
+
+namespace parse_print_tests {
+
+struct Case {
+    const char* string;
+    Attribute value;
+};
+
+inline std::ostream& operator<<(std::ostream& out, Case c) {
+    return out << "'" << std::string(c.string) << "'";
+}
+
+static constexpr Case kValidCases[] = {
+    {"align", Attribute::kAlign},
+    {"binding", Attribute::kBinding},
+    {"builtin", Attribute::kBuiltin},
+    {"compute", Attribute::kCompute},
+    {"diagnostic", Attribute::kDiagnostic},
+    {"fragment", Attribute::kFragment},
+    {"group", Attribute::kGroup},
+    {"id", Attribute::kId},
+    {"interpolate", Attribute::kInterpolate},
+    {"invariant", Attribute::kInvariant},
+    {"location", Attribute::kLocation},
+    {"must_use", Attribute::kMustUse},
+    {"size", Attribute::kSize},
+    {"vertex", Attribute::kVertex},
+    {"workgroup_size", Attribute::kWorkgroupSize},
+};
+
+static constexpr Case kInvalidCases[] = {
+    {"alccn", Attribute::kUndefined},
+    {"3g", Attribute::kUndefined},
+    {"aVign", Attribute::kUndefined},
+    {"bind1ng", Attribute::kUndefined},
+    {"bqnJing", Attribute::kUndefined},
+    {"bindin7ll", Attribute::kUndefined},
+    {"ppqqiliHH", Attribute::kUndefined},
+    {"bucv", Attribute::kUndefined},
+    {"biltGn", Attribute::kUndefined},
+    {"compiive", Attribute::kUndefined},
+    {"8WWmpute", Attribute::kUndefined},
+    {"cxxpute", Attribute::kUndefined},
+    {"dXagnosigg", Attribute::kUndefined},
+    {"dagnXuVc", Attribute::kUndefined},
+    {"diagnosti3", Attribute::kUndefined},
+    {"fraEment", Attribute::kUndefined},
+    {"PPagTTent", Attribute::kUndefined},
+    {"xxragddnt", Attribute::kUndefined},
+    {"g44oup", Attribute::kUndefined},
+    {"grVVSSp", Attribute::kUndefined},
+    {"22RRp", Attribute::kUndefined},
+    {"d", Attribute::kUndefined},
+    {"i", Attribute::kUndefined},
+    {"OVd", Attribute::kUndefined},
+    {"inyerpolae", Attribute::kUndefined},
+    {"rrnterpolll77Ge", Attribute::kUndefined},
+    {"inte4pol00te", Attribute::kUndefined},
+    {"inoornt", Attribute::kUndefined},
+    {"inzzriat", Attribute::kUndefined},
+    {"n11pariiin", Attribute::kUndefined},
+    {"XXocation", Attribute::kUndefined},
+    {"lIIc9955nnon", Attribute::kUndefined},
+    {"aaoHHatioYSS", Attribute::kUndefined},
+    {"mkksue", Attribute::kUndefined},
+    {"gjs_RRs", Attribute::kUndefined},
+    {"msb_se", Attribute::kUndefined},
+    {"jize", Attribute::kUndefined},
+    {"sze", Attribute::kUndefined},
+    {"qz", Attribute::kUndefined},
+    {"vNNtex", Attribute::kUndefined},
+    {"vevvx", Attribute::kUndefined},
+    {"veQQex", Attribute::kUndefined},
+    {"workgrrupffie", Attribute::kUndefined},
+    {"workgroup_sije", Attribute::kUndefined},
+    {"workgoupNNwsiz8", Attribute::kUndefined},
+};
+
+using AttributeParseTest = testing::TestWithParam<Case>;
+
+TEST_P(AttributeParseTest, Parse) {
+    const char* string = GetParam().string;
+    Attribute expect = GetParam().value;
+    EXPECT_EQ(expect, ParseAttribute(string));
+}
+
+INSTANTIATE_TEST_SUITE_P(ValidCases, AttributeParseTest, testing::ValuesIn(kValidCases));
+INSTANTIATE_TEST_SUITE_P(InvalidCases, AttributeParseTest, testing::ValuesIn(kInvalidCases));
+
+using AttributePrintTest = testing::TestWithParam<Case>;
+
+TEST_P(AttributePrintTest, Print) {
+    Attribute value = GetParam().value;
+    const char* expect = GetParam().string;
+    EXPECT_EQ(expect, utils::ToString(value));
+}
+
+INSTANTIATE_TEST_SUITE_P(ValidCases, AttributePrintTest, testing::ValuesIn(kValidCases));
+
+}  // namespace parse_print_tests
+
+}  // namespace
+}  // namespace tint::builtin

src/tint/builtin/attribute_test.cc.tmpl

@@ -0,0 +1,31 @@
+{{- /*
+--------------------------------------------------------------------------------
+Template file for use with tools/src/cmd/gen to generate attribute_test.cc
+
+To update the generated file, run:
+    ./tools/run gen
+
+See:
+* tools/src/cmd/gen for structures used by this template
+* https://golang.org/pkg/text/template/ for documentation on the template syntax
+--------------------------------------------------------------------------------
+*/ -}}
+
+{{- Import "src/tint/templates/enums.tmpl.inc" -}}
+{{- $enum := (Sem.Enum "attribute") -}}
+
+#include "src/tint/builtin/attribute.h"
+
+#include <gtest/gtest.h>
+
+#include <string>
+
+#include "src/tint/utils/string.h"
+
+namespace tint::builtin {
+namespace {
+
+{{ Eval "TestParsePrintEnum" $enum}}
+
+}  // namespace
+}  // namespace tint::builtin

src/tint/intrinsics.def

@@ -211,6 +211,29 @@ enum builtin_type {
   texture_external
 }
 
+// https://gpuweb.github.io/gpuweb/wgsl/#attributes
+// Notes:
+//  * `diagnostic` is listed, even though it is a keyword, so it appears in suggested alternatives
+//  * `const` is not listed here as it can not be written in user programs and should not show up
+//     in error messages
+enum attribute {
+  align
+  binding
+  builtin
+  compute
+  diagnostic
+  fragment
+  group
+  id
+  interpolate
+  invariant
+  location
+  must_use
+  size
+  vertex
+  workgroup_size
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // WGSL primitive types                                                       //
 // Types may be decorated with @precedence(N) to prioritize which type        //

src/tint/reader/wgsl/parser_impl.cc

@@ -34,6 +34,7 @@
 #include "src/tint/ast/unary_op_expression.h"
 #include "src/tint/ast/variable_decl_statement.h"
 #include "src/tint/ast/workgroup_attribute.h"
+#include "src/tint/builtin/attribute.h"
 #include "src/tint/reader/wgsl/classify_template_args.h"
 #include "src/tint/reader/wgsl/lexer.h"
 #include "src/tint/type/depth_texture.h"
@@ -2988,9 +2989,12 @@ Expect<const ast::Attribute*> ParserImpl::expect_attribute() {
 Maybe<const ast::Attribute*> ParserImpl::attribute() {
     // Note, the ATTR is matched by the called `attribute_list` in this case, so it is not matched
     // here and this has to be an attribute.
-    auto& t = next();
+    auto& t = peek();
 
-    if (t.Is(Token::Type::kDiagnostic)) {
+    if (match(Token::Type::kConst)) {
+        return add_error(t.source(), "const attribute may not appear in shaders");
+    }
+    if (match(Token::Type::kDiagnostic)) {
         auto control = expect_diagnostic_control();
         if (control.errored) {
             return Failure::kErrored;
@@ -2998,21 +3002,41 @@ Maybe<const ast::Attribute*> ParserImpl::attribute() {
         return create<ast::DiagnosticAttribute>(t.source(), std::move(control.value));
     }
 
-    if (!t.IsIdentifier()) {
+    auto attr = expect_enum("attribute", builtin::ParseAttribute, builtin::kAttributeStrings);
-        return Failure::kNoMatch;
+    if (attr.errored) {
+        return Failure::kErrored;
     }
 
-    utils::Vector<const ast::Expression*, 2> exprs;
+    uint32_t min = 1;
-    auto parse = [&](uint32_t min, uint32_t max) -> Maybe<bool> {
+    uint32_t max = 1;
+    switch (attr.value) {
+        case builtin::Attribute::kCompute:
+        case builtin::Attribute::kFragment:
+        case builtin::Attribute::kInvariant:
+        case builtin::Attribute::kMustUse:
+        case builtin::Attribute::kVertex:
+            min = 0;
+            max = 0;
+            break;
+        case builtin::Attribute::kInterpolate:
+            max = 2;
+            break;
+        case builtin::Attribute::kWorkgroupSize:
+            max = 3;
+            break;
+        default:
+            break;
+    }
+
+    utils::Vector<const ast::Expression*, 2> args;
+
     // Handle no parameter items which should have no parens
     if (min == 0) {
-            auto& p = peek();
+        auto& t2 = peek();
-            if (p.Is(Token::Type::kParenLeft)) {
+        if (match(Token::Type::kParenLeft)) {
-                return add_error(p.source(), t.to_str() + " attribute doesn't take parenthesis");
+            return add_error(t2.source(), t.to_str() + " attribute doesn't take parenthesis");
         }
-            return true;
+    } else {
-        }
-
         auto res = expect_paren_block(t.to_str() + " attribute", [&]() -> Expect<bool> {
             while (continue_parsing()) {
                 if (peek().Is(Token::Type::kParenRight)) {
@@ -3023,7 +3047,7 @@ Maybe<const ast::Attribute*> ParserImpl::attribute() {
                 if (expr.errored) {
                     return Failure::kErrored;
                 }
-                exprs.Push(expr.value);
+                args.Push(expr.value);
 
                 if (!match(Token::Type::kComma)) {
                     break;
@@ -3035,138 +3059,54 @@ Maybe<const ast::Attribute*> ParserImpl::attribute() {
             return Failure::kErrored;
         }
 
-        if (exprs.IsEmpty() || exprs.Length() < min) {
+        if (args.IsEmpty() || args.Length() < min) {
             return add_error(t.source(),
                              t.to_str() + " expects" + (min != max ? " at least " : " ") +
-                                 std::to_string(min) + " argument" + (min > 1 ? "s" : ""));
+                                 std::to_string(min) + " argument" + (min != 1 ? "s" : ""));
         }
-
+        if (args.Length() > max) {
-        if (exprs.Length() > max) {
             return add_error(t.source(),
                              t.to_str() + " expects" + (min != max ? " at most " : " ") +
-                                 std::to_string(max) + " argument" + (max > 1 ? "s" : "") +
+                                 std::to_string(max) + " argument" + (max != 1 ? "s" : "") +
-                                 ", got " + std::to_string(exprs.Length()));
+                                 ", got " + std::to_string(args.Length()));
         }
-        return true;
-    };
-
-    if (t == "align") {
-        auto res = parse(1, 1);
-        if (res.errored) {
-            return Failure::kErrored;
-        }
-        return create<ast::StructMemberAlignAttribute>(t.source(), exprs[0]);
     }
 
-    if (t == "binding") {
+    switch (attr.value) {
-        auto res = parse(1, 1);
+        case builtin::Attribute::kAlign:
-        if (res.errored) {
+            return create<ast::StructMemberAlignAttribute>(t.source(), args[0]);
-            return Failure::kErrored;
+        case builtin::Attribute::kBinding:
-        }
+            return create<ast::BindingAttribute>(t.source(), args[0]);
-        return create<ast::BindingAttribute>(t.source(), exprs[0]);
+        case builtin::Attribute::kBuiltin:
-    }
+            return create<ast::BuiltinAttribute>(t.source(), args[0]);
-
+        case builtin::Attribute::kCompute:
-    if (t == "builtin") {
-        auto res = parse(1, 1);
-        if (res.errored) {
-            return Failure::kErrored;
-        }
-        return create<ast::BuiltinAttribute>(t.source(), exprs[0]);
-    }
-
-    if (t == "compute") {
-        auto res = parse(0, 0);
-        if (res.errored) {
-            return Failure::kErrored;
-        }
             return create<ast::StageAttribute>(t.source(), ast::PipelineStage::kCompute);
-    }
+        case builtin::Attribute::kFragment:
-
-    // Note, `const` is not valid in a WGSL source file, it's internal only
-
-    if (t == "fragment") {
-        auto res = parse(0, 0);
-        if (res.errored) {
-            return Failure::kErrored;
-        }
             return create<ast::StageAttribute>(t.source(), ast::PipelineStage::kFragment);
-    }
+        case builtin::Attribute::kGroup:
-
+            return create<ast::GroupAttribute>(t.source(), args[0]);
-    if (t == "group") {
+        case builtin::Attribute::kId:
-        auto res = parse(1, 1);
+            return create<ast::IdAttribute>(t.source(), args[0]);
-        if (res.errored) {
+        case builtin::Attribute::kInterpolate:
-            return Failure::kErrored;
+            return create<ast::InterpolateAttribute>(t.source(), args[0],
-        }
+                                                     args.Length() == 2 ? args[1] : nullptr);
-        return create<ast::GroupAttribute>(t.source(), exprs[0]);
+        case builtin::Attribute::kInvariant:
-    }
-
-    if (t == "id") {
-        auto res = parse(1, 1);
-        if (res.errored) {
-            return Failure::kErrored;
-        }
-        return create<ast::IdAttribute>(t.source(), exprs[0]);
-    }
-
-    if (t == "interpolate") {
-        auto res = parse(1, 2);
-        if (res.errored) {
-            return Failure::kErrored;
-        }
-        return create<ast::InterpolateAttribute>(t.source(), exprs[0],
-                                                 exprs.Length() == 2 ? exprs[1] : nullptr);
-    }
-
-    if (t == "invariant") {
-        auto res = parse(0, 0);
-        if (res.errored) {
-            return Failure::kErrored;
-        }
             return create<ast::InvariantAttribute>(t.source());
-    }
+        case builtin::Attribute::kLocation:
-
+            return builder_.Location(t.source(), args[0]);
-    if (t == "location") {
+        case builtin::Attribute::kMustUse:
-        auto res = parse(1, 1);
-        if (res.errored) {
-            return Failure::kErrored;
-        }
-        return builder_.Location(t.source(), exprs[0]);
-    }
-
-    if (t == "must_use") {
-        auto res = parse(0, 0);
-        if (res.errored) {
-            return Failure::kErrored;
-        }
             return create<ast::MustUseAttribute>(t.source());
-    }
+        case builtin::Attribute::kSize:
-
+            return builder_.MemberSize(t.source(), args[0]);
-    if (t == "size") {
+        case builtin::Attribute::kVertex:
-        auto res = parse(1, 1);
-        if (res.errored) {
-            return Failure::kErrored;
-        }
-        return builder_.MemberSize(t.source(), exprs[0]);
-    }
-
-    if (t == "vertex") {
-        auto res = parse(0, 0);
-        if (res.errored) {
-            return Failure::kErrored;
-        }
             return create<ast::StageAttribute>(t.source(), ast::PipelineStage::kVertex);
-    }
+        case builtin::Attribute::kWorkgroupSize:
-
+            return create<ast::WorkgroupAttribute>(t.source(), args[0],
-    if (t == "workgroup_size") {
+                                                   args.Length() > 1 ? args[1] : nullptr,
-        auto res = parse(1, 3);
+                                                   args.Length() > 2 ? args[2] : nullptr);
-        if (res.errored) {
+        default:
-            return Failure::kErrored;
-        }
-        return create<ast::WorkgroupAttribute>(t.source(), exprs[0],
-                                               exprs.Length() > 1 ? exprs[1] : nullptr,
-                                               exprs.Length() > 2 ? exprs[2] : nullptr);
-    }
             return Failure::kNoMatch;
+    }
 }
 
 bool ParserImpl::expect_attributes_consumed(utils::VectorRef<const ast::Attribute*> in) {

src/tint/reader/wgsl/parser_impl_error_msg_test.cc

@@ -59,6 +59,14 @@ TEST_F(ParserImplErrorTest, AliasDeclInvalidAttribute) {
 )");
 }
 
+TEST_F(ParserImplErrorTest, ConstAttributeInvalid) {
+    EXPECT("@const fn main() { }",
+           R"(test.wgsl:1:2 error: const attribute may not appear in shaders
+@const fn main() { }
+ ^^^^^
+)");
+}
+
 TEST_F(ParserImplErrorTest, IndexExprInvalidExpr) {
     EXPECT("fn f() { x = y[^]; }",
            R"(test.wgsl:1:16 error: unable to parse expression inside []

src/tint/reader/wgsl/parser_impl_error_resync_test.cc

@@ -54,12 +54,9 @@ fn x(.) {}
      ^
 
 test.wgsl:4:2 error: expected attribute
+Possible values: 'align', 'binding', 'builtin', 'compute', 'diagnostic', 'fragment', 'group', 'id', 'interpolate', 'invariant', 'location', 'must_use', 'size', 'vertex', 'workgroup_size'
 @_ fn -> {}
  ^
-
-test.wgsl:4:7 error: expected identifier for function declaration
-@_ fn -> {}
-      ^^
 )");
 }
 
@@ -125,6 +122,7 @@ test.wgsl:5:10 error: expected ':' for struct member
          ^^^^
 
 test.wgsl:7:6 error: expected attribute
+Possible values: 'align', 'binding', 'builtin', 'compute', 'diagnostic', 'fragment', 'group', 'id', 'interpolate', 'invariant', 'location', 'must_use', 'size', 'vertex', 'workgroup_size'
     @- x : i32,
      ^
 )");

src/tint/reader/wgsl/parser_impl_function_attribute_list_test.cc

@@ -51,7 +51,9 @@ TEST_F(ParserImplTest, AttributeList_Invalid) {
     EXPECT_TRUE(attrs.errored);
     EXPECT_FALSE(attrs.matched);
     EXPECT_TRUE(attrs.value.IsEmpty());
-    EXPECT_EQ(p->error(), "1:2: expected attribute");
+    EXPECT_EQ(p->error(), R"(1:2: expected attribute
+Did you mean 'invariant'?
+Possible values: 'align', 'binding', 'builtin', 'compute', 'diagnostic', 'fragment', 'group', 'id', 'interpolate', 'invariant', 'location', 'must_use', 'size', 'vertex', 'workgroup_size')");
 }
 
 }  // namespace

src/tint/reader/wgsl/parser_impl_variable_attribute_list_test.cc

@@ -50,7 +50,9 @@ TEST_F(ParserImplTest, AttributeList_Invalid) {
     EXPECT_TRUE(attrs.errored);
     EXPECT_FALSE(attrs.matched);
     EXPECT_TRUE(attrs.value.IsEmpty());
-    EXPECT_EQ(p->error(), R"(1:2: expected attribute)");
+    EXPECT_EQ(p->error(), R"(1:2: expected attribute
+Did you mean 'invariant'?
+Possible values: 'align', 'binding', 'builtin', 'compute', 'diagnostic', 'fragment', 'group', 'id', 'interpolate', 'invariant', 'location', 'must_use', 'size', 'vertex', 'workgroup_size')");
 }
 
 }  // namespace