From 077d97a387bf41700fadccbc2b95df24162682b8 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Thu, 23 Feb 2023 19:08:57 +0000 Subject: [PATCH] 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 Commit-Queue: Dan Sinclair Kokoro: Kokoro --- src/tint/BUILD.gn | 3 + src/tint/CMakeLists.txt | 1 + src/tint/builtin/attribute.cc | 117 ++++++++++ src/tint/builtin/attribute.cc.tmpl | 25 ++ src/tint/builtin/attribute.h | 72 ++++++ src/tint/builtin/attribute.h.tmpl | 33 +++ src/tint/builtin/attribute_bench.cc | 151 ++++++++++++ src/tint/builtin/attribute_bench.cc.tmpl | 29 +++ src/tint/builtin/attribute_test.cc | 135 +++++++++++ src/tint/builtin/attribute_test.cc.tmpl | 31 +++ src/tint/intrinsics.def | 23 ++ src/tint/reader/wgsl/parser_impl.cc | 216 +++++++----------- .../reader/wgsl/parser_impl_error_msg_test.cc | 8 + .../wgsl/parser_impl_error_resync_test.cc | 6 +- ...arser_impl_function_attribute_list_test.cc | 4 +- ...arser_impl_variable_attribute_list_test.cc | 4 +- 16 files changed, 714 insertions(+), 144 deletions(-) create mode 100644 src/tint/builtin/attribute.cc create mode 100644 src/tint/builtin/attribute.cc.tmpl create mode 100644 src/tint/builtin/attribute.h create mode 100644 src/tint/builtin/attribute.h.tmpl create mode 100644 src/tint/builtin/attribute_bench.cc create mode 100644 src/tint/builtin/attribute_bench.cc.tmpl create mode 100644 src/tint/builtin/attribute_test.cc create mode 100644 src/tint/builtin/attribute_test.cc.tmpl diff --git a/src/tint/BUILD.gn b/src/tint/BUILD.gn index 4bcfdfd185..a93678f7fb 100644 --- a/src/tint/BUILD.gn +++ b/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", diff --git a/src/tint/CMakeLists.txt b/src/tint/CMakeLists.txt index fff86d0d17..a3835b7379 100644 --- a/src/tint/CMakeLists.txt +++ b/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) diff --git a/src/tint/builtin/attribute.cc b/src/tint/builtin/attribute.cc new file mode 100644 index 0000000000..d20b77c9fb --- /dev/null +++ b/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 << ""; +} + +} // namespace tint::builtin diff --git a/src/tint/builtin/attribute.cc.tmpl b/src/tint/builtin/attribute.cc.tmpl new file mode 100644 index 0000000000..dde123aa6f --- /dev/null +++ b/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 diff --git a/src/tint/builtin/attribute.h b/src/tint/builtin/attribute.h new file mode 100644 index 0000000000..bf9545879e --- /dev/null +++ b/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 + +/// \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_ diff --git a/src/tint/builtin/attribute.h.tmpl b/src/tint/builtin/attribute.h.tmpl new file mode 100644 index 0000000000..32ca6a3ad3 --- /dev/null +++ b/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 + +/// \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_ diff --git a/src/tint/builtin/attribute_bench.cc b/src/tint/builtin/attribute_bench.cc new file mode 100644 index 0000000000..5d12ab3200 --- /dev/null +++ b/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 + +#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 diff --git a/src/tint/builtin/attribute_bench.cc.tmpl b/src/tint/builtin/attribute_bench.cc.tmpl new file mode 100644 index 0000000000..8f65de0071 --- /dev/null +++ b/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 + +#include "benchmark/benchmark.h" + +namespace tint::builtin { +namespace { + +{{ Eval "BenchmarkParseEnum" $enum }} + +} // namespace +} // namespace tint::builtin diff --git a/src/tint/builtin/attribute_test.cc b/src/tint/builtin/attribute_test.cc new file mode 100644 index 0000000000..991946777e --- /dev/null +++ b/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 + +#include + +#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; + +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; + +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 diff --git a/src/tint/builtin/attribute_test.cc.tmpl b/src/tint/builtin/attribute_test.cc.tmpl new file mode 100644 index 0000000000..27dd8a9874 --- /dev/null +++ b/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 + +#include + +#include "src/tint/utils/string.h" + +namespace tint::builtin { +namespace { + +{{ Eval "TestParsePrintEnum" $enum}} + +} // namespace +} // namespace tint::builtin diff --git a/src/tint/intrinsics.def b/src/tint/intrinsics.def index 45b4b040b0..a89eab5cc5 100644 --- a/src/tint/intrinsics.def +++ b/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 // diff --git a/src/tint/reader/wgsl/parser_impl.cc b/src/tint/reader/wgsl/parser_impl.cc index 6b4b8d7d9e..13b47414c0 100644 --- a/src/tint/reader/wgsl/parser_impl.cc +++ b/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 ParserImpl::expect_attribute() { Maybe 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 ParserImpl::attribute() { return create(t.source(), std::move(control.value)); } - if (!t.IsIdentifier()) { - return Failure::kNoMatch; + auto attr = expect_enum("attribute", builtin::ParseAttribute, builtin::kAttributeStrings); + if (attr.errored) { + return Failure::kErrored; } - utils::Vector exprs; - auto parse = [&](uint32_t min, uint32_t max) -> Maybe { - // Handle no parameter items which should have no parens - if (min == 0) { - auto& p = peek(); - if (p.Is(Token::Type::kParenLeft)) { - return add_error(p.source(), t.to_str() + " attribute doesn't take parenthesis"); - } - return true; - } + uint32_t min = 1; + 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 args; + + // Handle no parameter items which should have no parens + if (min == 0) { + auto& t2 = peek(); + if (match(Token::Type::kParenLeft)) { + return add_error(t2.source(), t.to_str() + " attribute doesn't take parenthesis"); + } + } else { auto res = expect_paren_block(t.to_str() + " attribute", [&]() -> Expect { while (continue_parsing()) { if (peek().Is(Token::Type::kParenRight)) { @@ -3023,7 +3047,7 @@ Maybe 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 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 (exprs.Length() > max) { + if (args.Length() > max) { return add_error(t.source(), t.to_str() + " expects" + (min != max ? " at most " : " ") + - std::to_string(max) + " argument" + (max > 1 ? "s" : "") + - ", got " + std::to_string(exprs.Length())); + std::to_string(max) + " argument" + (max != 1 ? "s" : "") + + ", got " + std::to_string(args.Length())); } - return true; - }; - - if (t == "align") { - auto res = parse(1, 1); - if (res.errored) { - return Failure::kErrored; - } - return create(t.source(), exprs[0]); } - if (t == "binding") { - auto res = parse(1, 1); - if (res.errored) { - return Failure::kErrored; - } - return create(t.source(), exprs[0]); + switch (attr.value) { + case builtin::Attribute::kAlign: + return create(t.source(), args[0]); + case builtin::Attribute::kBinding: + return create(t.source(), args[0]); + case builtin::Attribute::kBuiltin: + return create(t.source(), args[0]); + case builtin::Attribute::kCompute: + return create(t.source(), ast::PipelineStage::kCompute); + case builtin::Attribute::kFragment: + return create(t.source(), ast::PipelineStage::kFragment); + case builtin::Attribute::kGroup: + return create(t.source(), args[0]); + case builtin::Attribute::kId: + return create(t.source(), args[0]); + case builtin::Attribute::kInterpolate: + return create(t.source(), args[0], + args.Length() == 2 ? args[1] : nullptr); + case builtin::Attribute::kInvariant: + return create(t.source()); + case builtin::Attribute::kLocation: + return builder_.Location(t.source(), args[0]); + case builtin::Attribute::kMustUse: + return create(t.source()); + case builtin::Attribute::kSize: + return builder_.MemberSize(t.source(), args[0]); + case builtin::Attribute::kVertex: + return create(t.source(), ast::PipelineStage::kVertex); + case builtin::Attribute::kWorkgroupSize: + return create(t.source(), args[0], + args.Length() > 1 ? args[1] : nullptr, + args.Length() > 2 ? args[2] : nullptr); + default: + return Failure::kNoMatch; } - - if (t == "builtin") { - auto res = parse(1, 1); - if (res.errored) { - return Failure::kErrored; - } - return create(t.source(), exprs[0]); - } - - if (t == "compute") { - auto res = parse(0, 0); - if (res.errored) { - return Failure::kErrored; - } - return create(t.source(), ast::PipelineStage::kCompute); - } - - // 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(t.source(), ast::PipelineStage::kFragment); - } - - if (t == "group") { - auto res = parse(1, 1); - if (res.errored) { - return Failure::kErrored; - } - return create(t.source(), exprs[0]); - } - - if (t == "id") { - auto res = parse(1, 1); - if (res.errored) { - return Failure::kErrored; - } - return create(t.source(), exprs[0]); - } - - if (t == "interpolate") { - auto res = parse(1, 2); - if (res.errored) { - return Failure::kErrored; - } - return create(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(t.source()); - } - - if (t == "location") { - 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(t.source()); - } - - if (t == "size") { - 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(t.source(), ast::PipelineStage::kVertex); - } - - if (t == "workgroup_size") { - auto res = parse(1, 3); - if (res.errored) { - return Failure::kErrored; - } - return create(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 in) { 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 ccee81866b..85e55cb518 100644 --- a/src/tint/reader/wgsl/parser_impl_error_msg_test.cc +++ b/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 [] diff --git a/src/tint/reader/wgsl/parser_impl_error_resync_test.cc b/src/tint/reader/wgsl/parser_impl_error_resync_test.cc index 9e37b51801..f3e306aef0 100644 --- a/src/tint/reader/wgsl/parser_impl_error_resync_test.cc +++ b/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, ^ )"); 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 785d23c4a4..ae43a0f4f3 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 @@ -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 diff --git a/src/tint/reader/wgsl/parser_impl_variable_attribute_list_test.cc b/src/tint/reader/wgsl/parser_impl_variable_attribute_list_test.cc index 24ad0204c6..07775c5b70 100644 --- a/src/tint/reader/wgsl/parser_impl_variable_attribute_list_test.cc +++ b/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