reader/wgsl: Improve reserved keyword error messages
And add `vec` and `mat` to the reserved keyword list (see https://github.com/gpuweb/gpuweb/pull/1896)
Move these reserved keyword checks out of the lexer and into the parser.
Generate a sensible error message.
Add tests.
Change-Id: I1876448201a2fd773f38f337b4e49bc61a7642f7
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/56545
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 5ef7d5c..7bde86a 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -762,6 +762,7 @@
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
reader/wgsl/parser_impl_sampled_texture_type_test.cc
reader/wgsl/parser_impl_sampler_type_test.cc
reader/wgsl/parser_impl_shift_expression_test.cc
diff --git a/src/reader/wgsl/lexer.cc b/src/reader/wgsl/lexer.cc
index 67d88d9..8c92c14 100644
--- a/src/reader/wgsl/lexer.cc
+++ b/src/reader/wgsl/lexer.cc
@@ -309,14 +309,9 @@
}
auto str = content_->data.substr(s, pos_ - s);
- auto t = check_reserved(source, str);
- if (!t.IsUninitialized()) {
- return t;
- }
-
end_source(source);
- t = check_keyword(source, str);
+ auto t = check_keyword(source, str);
if (!t.IsUninitialized()) {
return t;
}
@@ -691,48 +686,6 @@
return {};
}
-Token Lexer::check_reserved(const Source& source, const std::string& str) {
- if (str == "asm")
- return {Token::Type::kReservedKeyword, source, "asm"};
- if (str == "bf16")
- return {Token::Type::kReservedKeyword, source, "bf16"};
- if (str == "const")
- return {Token::Type::kReservedKeyword, source, "const"};
- if (str == "do")
- return {Token::Type::kReservedKeyword, source, "do"};
- if (str == "enum")
- return {Token::Type::kReservedKeyword, source, "enum"};
- if (str == "f16")
- return {Token::Type::kReservedKeyword, source, "f16"};
- if (str == "f64")
- return {Token::Type::kReservedKeyword, source, "f64"};
- if (str == "handle")
- return {Token::Type::kReservedKeyword, source, "handle"};
- if (str == "i8")
- return {Token::Type::kReservedKeyword, source, "i8"};
- if (str == "i16")
- return {Token::Type::kReservedKeyword, source, "i16"};
- if (str == "i64")
- return {Token::Type::kReservedKeyword, source, "i64"};
- if (str == "premerge")
- return {Token::Type::kReservedKeyword, source, "premerge"};
- if (str == "regardless")
- return {Token::Type::kReservedKeyword, source, "regardless"};
- if (str == "typedef")
- return {Token::Type::kReservedKeyword, source, "typedef"};
- if (str == "u8")
- return {Token::Type::kReservedKeyword, source, "u8"};
- if (str == "u16")
- return {Token::Type::kReservedKeyword, source, "u16"};
- if (str == "u64")
- return {Token::Type::kReservedKeyword, source, "u64"};
- if (str == "unless")
- return {Token::Type::kReservedKeyword, source, "unless"};
- if (str == "void")
- return {Token::Type::kReservedKeyword, source, "void"};
- return {};
-}
-
} // namespace wgsl
} // namespace reader
} // namespace tint
diff --git a/src/reader/wgsl/lexer.h b/src/reader/wgsl/lexer.h
index cec632e..b1774e9 100644
--- a/src/reader/wgsl/lexer.h
+++ b/src/reader/wgsl/lexer.h
@@ -45,7 +45,6 @@
size_t end,
int32_t base);
Token check_keyword(const Source&, const std::string&);
- Token check_reserved(const Source&, const std::string&);
Token try_float();
Token try_hex_integer();
Token try_ident();
diff --git a/src/reader/wgsl/lexer_test.cc b/src/reader/wgsl/lexer_test.cc
index 4134f1d6..bfcc1a7 100644
--- a/src/reader/wgsl/lexer_test.cc
+++ b/src/reader/wgsl/lexer_test.cc
@@ -523,38 +523,6 @@
TokenData{"vec4", Token::Type::kVec4},
TokenData{"workgroup", Token::Type::kWorkgroup}));
-using KeywordTest_Reserved = testing::TestWithParam<const char*>;
-TEST_P(KeywordTest_Reserved, Parses) {
- auto* keyword = GetParam();
- Source::FileContent content(keyword);
- Lexer l("test.wgsl", &content);
-
- auto t = l.next();
- EXPECT_TRUE(t.IsReservedKeyword());
- EXPECT_EQ(t.to_str(), keyword);
-}
-INSTANTIATE_TEST_SUITE_P(LexerTest,
- KeywordTest_Reserved,
- testing::Values("asm",
- "bf16",
- "const",
- "do",
- "enum",
- "f16",
- "f64",
- "handle",
- "i8",
- "i16",
- "i64",
- "premerge",
- "typedef",
- "u8",
- "u16",
- "u64",
- "unless",
- "regardless",
- "void"));
-
} // namespace
} // namespace wgsl
} // namespace reader
diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc
index c8385e5..7808f9c 100644
--- a/src/reader/wgsl/parser_impl.cc
+++ b/src/reader/wgsl/parser_impl.cc
@@ -121,8 +121,9 @@
const char kWorkgroupSizeDecoration[] = "workgroup_size";
bool is_decoration(Token t) {
- if (!t.IsIdentifier())
+ if (!t.IsIdentifier()) {
return false;
+ }
auto s = t.to_str();
return s == kAlignDecoration || s == kBindingDecoration ||
@@ -133,6 +134,17 @@
s == kStrideDecoration || s == kWorkgroupSizeDecoration;
}
+// https://gpuweb.github.io/gpuweb/wgsl.html#reserved-keywords
+bool is_reserved(Token t) {
+ auto s = t.to_str();
+ return s == "asm" || s == "bf16" || s == "const" || s == "do" ||
+ s == "enum" || s == "f16" || s == "f64" || s == "handle" ||
+ s == "i8" || s == "i16" || s == "i64" || s == "mat" ||
+ s == "premerge" || s == "regardless" || s == "typedef" || s == "u8" ||
+ s == "u16" || s == "u64" || s == "unless" || s == "using" ||
+ s == "vec" || s == "void" || s == "while";
+}
+
/// Enter-exit counters for block token types.
/// Used by sync_to() to skip over closing block tokens that were opened during
/// the forward scan.
@@ -3261,6 +3273,12 @@
return Failure::kErrored;
}
next();
+
+ if (is_reserved(t)) {
+ return add_error(t.source(),
+ "'" + t.to_str() + "' is a reserved keyword");
+ }
+
return {t.to_str(), t.source()};
}
synchronized_ = false;
diff --git a/src/reader/wgsl/parser_impl_reserved_keyword_test.cc b/src/reader/wgsl/parser_impl_reserved_keyword_test.cc
new file mode 100644
index 0000000..0b1ce39
--- /dev/null
+++ b/src/reader/wgsl/parser_impl_reserved_keyword_test.cc
@@ -0,0 +1,115 @@
+// Copyright 2021 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/reader/wgsl/parser_impl_test_helper.h"
+
+namespace tint {
+namespace reader {
+namespace wgsl {
+namespace {
+
+using ParserImplReservedKeywordTest = ParserImplTestWithParam<std::string>;
+TEST_P(ParserImplReservedKeywordTest, Function) {
+ auto name = GetParam();
+ auto p = parser("fn " + name + "() {}");
+ EXPECT_FALSE(p->Parse());
+ EXPECT_TRUE(p->has_error());
+ EXPECT_EQ(p->error(), "1:4: '" + name + "' is a reserved keyword");
+}
+TEST_P(ParserImplReservedKeywordTest, ModuleLet) {
+ auto name = GetParam();
+ auto p = parser("let " + name + " : i32 = 1;");
+ EXPECT_FALSE(p->Parse());
+ EXPECT_TRUE(p->has_error());
+ EXPECT_EQ(p->error(), "1:5: '" + name + "' is a reserved keyword");
+}
+TEST_P(ParserImplReservedKeywordTest, ModuleVar) {
+ auto name = GetParam();
+ auto p = parser("var " + name + " : i32 = 1;");
+ EXPECT_FALSE(p->Parse());
+ EXPECT_TRUE(p->has_error());
+ EXPECT_EQ(p->error(), "1:5: '" + name + "' is a reserved keyword");
+}
+TEST_P(ParserImplReservedKeywordTest, FunctionLet) {
+ auto name = GetParam();
+ auto p = parser("fn f() { let " + name + " : i32 = 1; }");
+ EXPECT_FALSE(p->Parse());
+ EXPECT_TRUE(p->has_error());
+ EXPECT_EQ(p->error(), "1:14: '" + name + "' is a reserved keyword");
+}
+TEST_P(ParserImplReservedKeywordTest, FunctionVar) {
+ auto name = GetParam();
+ auto p = parser("fn f() { var " + name + " : i32 = 1; }");
+ EXPECT_FALSE(p->Parse());
+ EXPECT_TRUE(p->has_error());
+ EXPECT_EQ(p->error(), "1:14: '" + name + "' is a reserved keyword");
+}
+TEST_P(ParserImplReservedKeywordTest, FunctionParam) {
+ auto name = GetParam();
+ auto p = parser("fn f(" + name + " : i32) {}");
+ EXPECT_FALSE(p->Parse());
+ EXPECT_TRUE(p->has_error());
+ EXPECT_EQ(p->error(), "1:6: '" + name + "' is a reserved keyword");
+}
+TEST_P(ParserImplReservedKeywordTest, Struct) {
+ auto name = GetParam();
+ auto p = parser("struct " + name + " {};");
+ EXPECT_FALSE(p->Parse());
+ EXPECT_TRUE(p->has_error());
+ EXPECT_EQ(p->error(), "1:8: '" + name + "' is a reserved keyword");
+}
+TEST_P(ParserImplReservedKeywordTest, StructMember) {
+ auto name = GetParam();
+ auto p = parser("struct S { " + name + " : i32; };");
+ EXPECT_FALSE(p->Parse());
+ EXPECT_TRUE(p->has_error());
+ EXPECT_EQ(p->error(), "1:12: '" + name + "' is a reserved keyword");
+}
+TEST_P(ParserImplReservedKeywordTest, Alias) {
+ auto name = GetParam();
+ auto p = parser("type " + name + " = i32;");
+ EXPECT_FALSE(p->Parse());
+ EXPECT_TRUE(p->has_error());
+ EXPECT_EQ(p->error(), "1:6: '" + name + "' is a reserved keyword");
+}
+INSTANTIATE_TEST_SUITE_P(ParserImplReservedKeywordTest,
+ ParserImplReservedKeywordTest,
+ testing::Values("asm",
+ "bf16",
+ "const",
+ "do",
+ "enum",
+ "f16",
+ "f64",
+ "handle",
+ "i8",
+ "i16",
+ "i64",
+ "mat",
+ "premerge",
+ "regardless",
+ "typedef",
+ "u8",
+ "u16",
+ "u64",
+ "unless",
+ "using",
+ "vec",
+ "void",
+ "while"));
+
+} // namespace
+} // namespace wgsl
+} // namespace reader
+} // namespace tint
diff --git a/src/reader/wgsl/token.cc b/src/reader/wgsl/token.cc
index acf7062..7616a68 100644
--- a/src/reader/wgsl/token.cc
+++ b/src/reader/wgsl/token.cc
@@ -23,8 +23,6 @@
switch (type) {
case Token::Type::kError:
return "kError";
- case Token::Type::kReservedKeyword:
- return "kReservedKeyword";
case Token::Type::kEOF:
return "kEOF";
case Token::Type::kIdentifier:
diff --git a/src/reader/wgsl/token.h b/src/reader/wgsl/token.h
index 2b949f5..893efaa 100644
--- a/src/reader/wgsl/token.h
+++ b/src/reader/wgsl/token.h
@@ -30,8 +30,6 @@
enum class Type {
/// Error result
kError = -2,
- /// Reserved keyword
- kReservedKeyword = -1,
/// Uninitialized token
kUninitialized = 0,
/// End of input string reached
@@ -367,7 +365,6 @@
/// @returns true if the token is uninitialized
bool IsUninitialized() const { return type_ == Type::kUninitialized; }
/// @returns true if the token is reserved
- bool IsReservedKeyword() const { return type_ == Type::kReservedKeyword; }
/// @returns true if the token is an error
bool IsError() const { return type_ == Type::kError; }
/// @returns true if the token is EOF
diff --git a/src/transform/inline_pointer_lets_test.cc b/src/transform/inline_pointer_lets_test.cc
index 51c1d89..91818b8 100644
--- a/src/transform/inline_pointer_lets_test.cc
+++ b/src/transform/inline_pointer_lets_test.cc
@@ -123,7 +123,7 @@
*p = 4;
}
-fn vec() {
+fn vector() {
var v : vec3<f32>;
var i : i32 = 0;
var j : i32 = 0;
@@ -132,7 +132,7 @@
*p = 4.0;
}
-fn mat() {
+fn matrix() {
var m : mat3x3<f32>;
var i : i32 = 0;
var j : i32 = 0;
@@ -156,7 +156,7 @@
*(&(a[p_save].i)) = 4;
}
-fn vec() {
+fn vector() {
var v : vec3<f32>;
var i : i32 = 0;
var j : i32 = 0;
@@ -165,7 +165,7 @@
*(&(v[p_save_1])) = 4.0;
}
-fn mat() {
+fn matrix() {
var m : mat3x3<f32>;
var i : i32 = 0;
var j : i32 = 0;
diff --git a/src/transform/renamer_test.cc b/src/transform/renamer_test.cc
index 73467a0..1c57b4a 100644
--- a/src/transform/renamer_test.cc
+++ b/src/transform/renamer_test.cc
@@ -838,14 +838,14 @@
"unorm",
"unroll",
"unsigned",
- "using",
+ // "using", // WGSL reserved keyword
"vector",
"vertexfragment",
"vertexshader",
"virtual",
// "void", // WGSL keyword
- "volatile",
- "while"));
+ "volatile"));
+// "while" // WGSL reserved keyword
INSTANTIATE_TEST_SUITE_P(RenamerTestMsl,
RenamerTestMsl,
@@ -928,12 +928,12 @@
"typename",
"union",
"unsigned",
- "using",
+ // "using", // WGSL reserved keyword
"virtual",
// "void", // Also used in WGSL
"volatile",
"wchar_t",
- "while",
+ // "while", // WGSL reserved keyword
"xor",
"xor_eq",
@@ -1087,7 +1087,7 @@
"ushort2",
"ushort3",
"ushort4",
- "vec",
+ // "vec", // WGSL reserved keyword
"vertex"));
} // namespace
diff --git a/test/BUILD.gn b/test/BUILD.gn
index 99f0e23..0431fa9 100644
--- a/test/BUILD.gn
+++ b/test/BUILD.gn
@@ -450,6 +450,7 @@
"../src/reader/wgsl/parser_impl_pipeline_stage_test.cc",
"../src/reader/wgsl/parser_impl_primary_expression_test.cc",
"../src/reader/wgsl/parser_impl_relational_expression_test.cc",
+ "../src/reader/wgsl/parser_impl_reserved_keyword_test.cc",
"../src/reader/wgsl/parser_impl_sampled_texture_type_test.cc",
"../src/reader/wgsl/parser_impl_sampler_type_test.cc",
"../src/reader/wgsl/parser_impl_shift_expression_test.cc",
