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>
This commit is contained in:
Ben Clayton 2021-07-05 21:48:37 +00:00 committed by Tint LUCI CQ
parent ffe7978dbf
commit 5d8eb4a758
11 changed files with 147 additions and 97 deletions

View File

@ -762,6 +762,7 @@ if(${TINT_BUILD_TESTS})
reader/wgsl/parser_impl_pipeline_stage_test.cc reader/wgsl/parser_impl_pipeline_stage_test.cc
reader/wgsl/parser_impl_primary_expression_test.cc reader/wgsl/parser_impl_primary_expression_test.cc
reader/wgsl/parser_impl_relational_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_sampled_texture_type_test.cc
reader/wgsl/parser_impl_sampler_type_test.cc reader/wgsl/parser_impl_sampler_type_test.cc
reader/wgsl/parser_impl_shift_expression_test.cc reader/wgsl/parser_impl_shift_expression_test.cc

View File

@ -309,14 +309,9 @@ Token Lexer::try_ident() {
} }
auto str = content_->data.substr(s, pos_ - s); auto str = content_->data.substr(s, pos_ - s);
auto t = check_reserved(source, str);
if (!t.IsUninitialized()) {
return t;
}
end_source(source); end_source(source);
t = check_keyword(source, str); auto t = check_keyword(source, str);
if (!t.IsUninitialized()) { if (!t.IsUninitialized()) {
return t; return t;
} }
@ -691,48 +686,6 @@ Token Lexer::check_keyword(const Source& source, const std::string& str) {
return {}; 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 wgsl
} // namespace reader } // namespace reader
} // namespace tint } // namespace tint

View File

@ -45,7 +45,6 @@ class Lexer {
size_t end, size_t end,
int32_t base); int32_t base);
Token check_keyword(const Source&, const std::string&); Token check_keyword(const Source&, const std::string&);
Token check_reserved(const Source&, const std::string&);
Token try_float(); Token try_float();
Token try_hex_integer(); Token try_hex_integer();
Token try_ident(); Token try_ident();

View File

@ -523,38 +523,6 @@ INSTANTIATE_TEST_SUITE_P(
TokenData{"vec4", Token::Type::kVec4}, TokenData{"vec4", Token::Type::kVec4},
TokenData{"workgroup", Token::Type::kWorkgroup})); 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
} // namespace wgsl } // namespace wgsl
} // namespace reader } // namespace reader

View File

@ -121,8 +121,9 @@ const char kStrideDecoration[] = "stride";
const char kWorkgroupSizeDecoration[] = "workgroup_size"; const char kWorkgroupSizeDecoration[] = "workgroup_size";
bool is_decoration(Token t) { bool is_decoration(Token t) {
if (!t.IsIdentifier()) if (!t.IsIdentifier()) {
return false; return false;
}
auto s = t.to_str(); auto s = t.to_str();
return s == kAlignDecoration || s == kBindingDecoration || return s == kAlignDecoration || s == kBindingDecoration ||
@ -133,6 +134,17 @@ bool is_decoration(Token t) {
s == kStrideDecoration || s == kWorkgroupSizeDecoration; 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. /// Enter-exit counters for block token types.
/// Used by sync_to() to skip over closing block tokens that were opened during /// Used by sync_to() to skip over closing block tokens that were opened during
/// the forward scan. /// the forward scan.
@ -3261,6 +3273,12 @@ Expect<std::string> ParserImpl::expect_ident(const std::string& use) {
return Failure::kErrored; return Failure::kErrored;
} }
next(); next();
if (is_reserved(t)) {
return add_error(t.source(),
"'" + t.to_str() + "' is a reserved keyword");
}
return {t.to_str(), t.source()}; return {t.to_str(), t.source()};
} }
synchronized_ = false; synchronized_ = false;

View File

@ -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

View File

@ -23,8 +23,6 @@ std::string Token::TypeToName(Type type) {
switch (type) { switch (type) {
case Token::Type::kError: case Token::Type::kError:
return "kError"; return "kError";
case Token::Type::kReservedKeyword:
return "kReservedKeyword";
case Token::Type::kEOF: case Token::Type::kEOF:
return "kEOF"; return "kEOF";
case Token::Type::kIdentifier: case Token::Type::kIdentifier:

View File

@ -30,8 +30,6 @@ class Token {
enum class Type { enum class Type {
/// Error result /// Error result
kError = -2, kError = -2,
/// Reserved keyword
kReservedKeyword = -1,
/// Uninitialized token /// Uninitialized token
kUninitialized = 0, kUninitialized = 0,
/// End of input string reached /// End of input string reached
@ -367,7 +365,6 @@ class Token {
/// @returns true if the token is uninitialized /// @returns true if the token is uninitialized
bool IsUninitialized() const { return type_ == Type::kUninitialized; } bool IsUninitialized() const { return type_ == Type::kUninitialized; }
/// @returns true if the token is reserved /// @returns true if the token is reserved
bool IsReservedKeyword() const { return type_ == Type::kReservedKeyword; }
/// @returns true if the token is an error /// @returns true if the token is an error
bool IsError() const { return type_ == Type::kError; } bool IsError() const { return type_ == Type::kError; }
/// @returns true if the token is EOF /// @returns true if the token is EOF

View File

@ -123,7 +123,7 @@ fn arr() {
*p = 4; *p = 4;
} }
fn vec() { fn vector() {
var v : vec3<f32>; var v : vec3<f32>;
var i : i32 = 0; var i : i32 = 0;
var j : i32 = 0; var j : i32 = 0;
@ -132,7 +132,7 @@ fn vec() {
*p = 4.0; *p = 4.0;
} }
fn mat() { fn matrix() {
var m : mat3x3<f32>; var m : mat3x3<f32>;
var i : i32 = 0; var i : i32 = 0;
var j : i32 = 0; var j : i32 = 0;
@ -156,7 +156,7 @@ fn arr() {
*(&(a[p_save].i)) = 4; *(&(a[p_save].i)) = 4;
} }
fn vec() { fn vector() {
var v : vec3<f32>; var v : vec3<f32>;
var i : i32 = 0; var i : i32 = 0;
var j : i32 = 0; var j : i32 = 0;
@ -165,7 +165,7 @@ fn vec() {
*(&(v[p_save_1])) = 4.0; *(&(v[p_save_1])) = 4.0;
} }
fn mat() { fn matrix() {
var m : mat3x3<f32>; var m : mat3x3<f32>;
var i : i32 = 0; var i : i32 = 0;
var j : i32 = 0; var j : i32 = 0;

View File

@ -838,14 +838,14 @@ INSTANTIATE_TEST_SUITE_P(RenamerTestHlsl,
"unorm", "unorm",
"unroll", "unroll",
"unsigned", "unsigned",
"using", // "using", // WGSL reserved keyword
"vector", "vector",
"vertexfragment", "vertexfragment",
"vertexshader", "vertexshader",
"virtual", "virtual",
// "void", // WGSL keyword // "void", // WGSL keyword
"volatile", "volatile"));
"while")); // "while" // WGSL reserved keyword
INSTANTIATE_TEST_SUITE_P(RenamerTestMsl, INSTANTIATE_TEST_SUITE_P(RenamerTestMsl,
RenamerTestMsl, RenamerTestMsl,
@ -928,12 +928,12 @@ INSTANTIATE_TEST_SUITE_P(RenamerTestMsl,
"typename", "typename",
"union", "union",
"unsigned", "unsigned",
"using", // "using", // WGSL reserved keyword
"virtual", "virtual",
// "void", // Also used in WGSL // "void", // Also used in WGSL
"volatile", "volatile",
"wchar_t", "wchar_t",
"while", // "while", // WGSL reserved keyword
"xor", "xor",
"xor_eq", "xor_eq",
@ -1087,7 +1087,7 @@ INSTANTIATE_TEST_SUITE_P(RenamerTestMsl,
"ushort2", "ushort2",
"ushort3", "ushort3",
"ushort4", "ushort4",
"vec", // "vec", // WGSL reserved keyword
"vertex")); "vertex"));
} // namespace } // namespace

View File

@ -450,6 +450,7 @@ tint_unittests_source_set("tint_unittests_wgsl_reader_src") {
"../src/reader/wgsl/parser_impl_pipeline_stage_test.cc", "../src/reader/wgsl/parser_impl_pipeline_stage_test.cc",
"../src/reader/wgsl/parser_impl_primary_expression_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_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_sampled_texture_type_test.cc",
"../src/reader/wgsl/parser_impl_sampler_type_test.cc", "../src/reader/wgsl/parser_impl_sampler_type_test.cc",
"../src/reader/wgsl/parser_impl_shift_expression_test.cc", "../src/reader/wgsl/parser_impl_shift_expression_test.cc",