tint/reader/wgsl: Improve errors when parsing extensions
If the extension name doesn't parse, then generate an error message that includes the list of possible values, and a suggestion if there was a close match. Change-Id: I0eb2a682ca5a0717bb31d2716824663924ccd8f2 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/105324 Commit-Queue: Ben Clayton <bclayton@chromium.org> Kokoro: Kokoro <noreply+kokoro@google.com> Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
This commit is contained in:
parent
760eee9e92
commit
ecd7f7ee7f
|
@ -371,40 +371,36 @@ Maybe<Void> ParserImpl::enable_directive() {
|
|||
}
|
||||
|
||||
// Match the extension name.
|
||||
Expect<std::string> name = {""};
|
||||
auto& t = peek();
|
||||
if (t.IsIdentifier()) {
|
||||
synchronized_ = true;
|
||||
next();
|
||||
name = {t.to_str(), t.source()};
|
||||
} else if (t.Is(Token::Type::kF16)) {
|
||||
// `f16` is a valid extension name and also a keyword
|
||||
synchronized_ = true;
|
||||
next();
|
||||
name = {"f16", t.source()};
|
||||
} else if (t.Is(Token::Type::kParenLeft)) {
|
||||
if (handle_error(t)) {
|
||||
// The token might itself be an error.
|
||||
return Failure::kErrored;
|
||||
}
|
||||
|
||||
if (t.Is(Token::Type::kParenLeft)) {
|
||||
// A common error case is writing `enable(foo);` instead of `enable foo;`.
|
||||
synchronized_ = false;
|
||||
return add_error(t.source(), "enable directives don't take parenthesis");
|
||||
} else if (handle_error(t)) {
|
||||
// The token might itself be an error.
|
||||
return Failure::kErrored;
|
||||
}
|
||||
|
||||
auto extension = ast::Extension::kInvalid;
|
||||
if (t.Is(Token::Type::kF16)) {
|
||||
// `f16` is a valid extension name and also a keyword
|
||||
synchronized_ = true;
|
||||
next();
|
||||
extension = ast::Extension::kF16;
|
||||
} else {
|
||||
// Failed to match an extension name.
|
||||
synchronized_ = false;
|
||||
return add_error(t.source(), "invalid extension name");
|
||||
auto ext = expect_enum("extension", ast::ParseExtension, ast::kExtensionStrings);
|
||||
if (ext.errored) {
|
||||
return Failure::kErrored;
|
||||
}
|
||||
extension = ext.value;
|
||||
}
|
||||
|
||||
if (!expect("enable directive", Token::Type::kSemicolon)) {
|
||||
return Failure::kErrored;
|
||||
}
|
||||
|
||||
auto extension = ast::ParseExtension(name.value);
|
||||
if (extension == ast::Extension::kInvalid) {
|
||||
return add_error(name.source, "unsupported extension: '" + name.value + "'");
|
||||
}
|
||||
builder_.AST().AddEnable(create<ast::Enable>(name.source, extension));
|
||||
|
||||
builder_.AST().AddEnable(create<ast::Enable>(t.source(), extension));
|
||||
return kSuccess;
|
||||
});
|
||||
|
||||
|
|
|
@ -61,7 +61,21 @@ TEST_F(EnableDirectiveTest, InvalidIdentifier) {
|
|||
p->enable_directive();
|
||||
// Error when unknown extension found
|
||||
EXPECT_TRUE(p->has_error());
|
||||
EXPECT_EQ(p->error(), "1:8: unsupported extension: 'NotAValidExtensionName'");
|
||||
EXPECT_EQ(p->error(), R"(1:8: expected extension
|
||||
Possible values: 'chromium_disable_uniformity_analysis', 'chromium_experimental_dp4a', 'chromium_experimental_push_constant', 'f16')");
|
||||
auto program = p->program();
|
||||
auto& ast = program.AST();
|
||||
EXPECT_EQ(ast.Enables().Length(), 0u);
|
||||
EXPECT_EQ(ast.GlobalDeclarations().Length(), 0u);
|
||||
}
|
||||
|
||||
TEST_F(EnableDirectiveTest, InvalidIdentifierSuggest) {
|
||||
auto p = parser("enable f15;");
|
||||
p->enable_directive();
|
||||
// Error when unknown extension found
|
||||
EXPECT_TRUE(p->has_error());
|
||||
EXPECT_EQ(p->error(), R"(1:8: expected extension. Did you mean 'f16'?
|
||||
Possible values: 'chromium_disable_uniformity_analysis', 'chromium_experimental_dp4a', 'chromium_experimental_push_constant', 'f16')");
|
||||
auto program = p->program();
|
||||
auto& ast = program.AST();
|
||||
EXPECT_EQ(ast.Enables().Length(), 0u);
|
||||
|
@ -108,7 +122,8 @@ TEST_F(EnableDirectiveTest, InvalidTokens) {
|
|||
auto p = parser("enable <f16;");
|
||||
p->translation_unit();
|
||||
EXPECT_TRUE(p->has_error());
|
||||
EXPECT_EQ(p->error(), "1:8: invalid extension name");
|
||||
EXPECT_EQ(p->error(), R"(1:8: expected extension
|
||||
Possible values: 'chromium_disable_uniformity_analysis', 'chromium_experimental_dp4a', 'chromium_experimental_push_constant', 'f16')");
|
||||
auto program = p->program();
|
||||
auto& ast = program.AST();
|
||||
EXPECT_EQ(ast.Enables().Length(), 0u);
|
||||
|
@ -118,7 +133,8 @@ TEST_F(EnableDirectiveTest, InvalidTokens) {
|
|||
auto p = parser("enable =;");
|
||||
p->translation_unit();
|
||||
EXPECT_TRUE(p->has_error());
|
||||
EXPECT_EQ(p->error(), "1:8: invalid extension name");
|
||||
EXPECT_EQ(p->error(), R"(1:8: expected extension
|
||||
Possible values: 'chromium_disable_uniformity_analysis', 'chromium_experimental_dp4a', 'chromium_experimental_push_constant', 'f16')");
|
||||
auto program = p->program();
|
||||
auto& ast = program.AST();
|
||||
EXPECT_EQ(ast.Enables().Length(), 0u);
|
||||
|
@ -128,7 +144,8 @@ TEST_F(EnableDirectiveTest, InvalidTokens) {
|
|||
auto p = parser("enable vec4;");
|
||||
p->translation_unit();
|
||||
EXPECT_TRUE(p->has_error());
|
||||
EXPECT_EQ(p->error(), "1:8: invalid extension name");
|
||||
EXPECT_EQ(p->error(), R"(1:8: expected extension
|
||||
Possible values: 'chromium_disable_uniformity_analysis', 'chromium_experimental_dp4a', 'chromium_experimental_push_constant', 'f16')");
|
||||
auto program = p->program();
|
||||
auto& ast = program.AST();
|
||||
EXPECT_EQ(ast.Enables().Length(), 0u);
|
||||
|
|
Loading…
Reference in New Issue