[wgsl] Parse RW storage texture language feature
Bug: tint:2088
Change-Id: I4ee9bde261bad9c3776f79e4b3dcb91472de6d99
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/158631
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
diff --git a/src/tint/lang/wgsl/reader/parser/parser.cc b/src/tint/lang/wgsl/reader/parser/parser.cc
index 40d1484..54aeb27 100644
--- a/src/tint/lang/wgsl/reader/parser/parser.cc
+++ b/src/tint/lang/wgsl/reader/parser/parser.cc
@@ -456,6 +456,7 @@
// : require identifier (COMMA identifier)* COMMA? SEMICOLON
Maybe<Void> Parser::requires_directive() {
return sync(Token::Type::kSemicolon, [&]() -> Maybe<Void> {
+ MultiTokenSource decl_source(this);
if (!match(Token::Type::kRequires)) {
return Failure::kNoMatch;
}
@@ -473,34 +474,43 @@
return add_error(t.source(), "requires directives don't take parenthesis");
}
+ wgsl::LanguageFeatures features;
while (continue_parsing()) {
- auto& t2 = peek();
-
- // Match the require name.
+ auto& t2 = next();
if (handle_error(t2)) {
// The token might itself be an error.
return Failure::kErrored;
}
+ // Match the require name.
if (t2.IsIdentifier()) {
- // TODO(dsinclair): When there are actual values for a requires directive they
- // should be checked here.
-
- // Any identifer is a valid feature name, so we correctly handle new feature
- // names getting added in the future, they just all get flagged as not supported.
- return add_error(t2.source(), "feature '" + t2.to_str() + "' is not supported");
- }
- if (t2.Is(Token::Type::kSemicolon)) {
- break;
- }
- if (!match(Token::Type::kComma)) {
+ auto feature = wgsl::ParseLanguageFeature(t2.to_str_view());
+ if (feature == LanguageFeature::kUndefined) {
+ // Any identifier is a valid feature name, so we correctly handle new feature
+ // names getting added in the future, they just all get flagged as not
+ // supported.
+ return add_error(t2.source(), "feature '" + t2.to_str() + "' is not supported");
+ }
+ features.Add(feature);
+ } else {
return add_error(t2.source(), "invalid feature name for requires");
}
+
+ if (!match(Token::Type::kComma)) {
+ break;
+ }
+ if (peek_is(Token::Type::kSemicolon)) {
+ break;
+ }
}
- // TODO(dsinclair): When there are actual values for a requires directive then the
- // `while` will need to keep track if any were seen, and this needs to become
- // conditional.
- return add_error(t.source(), "missing feature names in requires directive");
+
+ if (!expect("requires directive", Token::Type::kSemicolon)) {
+ return Failure::kErrored;
+ }
+
+ builder_.AST().AddRequires(
+ create<ast::Requires>(decl_source.Source(), std::move(features)));
+ return kSuccess;
});
}
diff --git a/src/tint/lang/wgsl/reader/parser/require_directive_test.cc b/src/tint/lang/wgsl/reader/parser/require_directive_test.cc
index 43ea6be..8e7ff03 100644
--- a/src/tint/lang/wgsl/reader/parser/require_directive_test.cc
+++ b/src/tint/lang/wgsl/reader/parser/require_directive_test.cc
@@ -33,11 +33,64 @@
using RequiresDirectiveTest = WGSLParserTest;
// Test a valid require directive.
-// There currently are no valid require directives
-TEST_F(RequiresDirectiveTest, DISABLED_Valid) {
- auto p = parser("requires <sometime>;");
+TEST_F(RequiresDirectiveTest, Single) {
+ auto p = parser("requires readonly_and_readwrite_storage_textures;");
p->requires_directive();
EXPECT_FALSE(p->has_error()) << p->error();
+
+ auto program = p->program();
+ auto& ast = program.AST();
+ ASSERT_EQ(ast.Requires().Length(), 1u);
+ auto* req = ast.Requires()[0];
+ EXPECT_EQ(req->source.range.begin.line, 1u);
+ EXPECT_EQ(req->source.range.begin.column, 1u);
+ EXPECT_EQ(req->source.range.end.line, 1u);
+ EXPECT_EQ(req->source.range.end.column, 50u);
+ ASSERT_EQ(req->features.Length(), 1u);
+ EXPECT_EQ(req->features[0], wgsl::LanguageFeature::kReadonlyAndReadwriteStorageTextures);
+ ASSERT_EQ(ast.GlobalDeclarations().Length(), 1u);
+ EXPECT_EQ(ast.GlobalDeclarations()[0], req);
+}
+
+// Test a valid require directive with a trailing comma.
+TEST_F(RequiresDirectiveTest, Single_TrailingComma) {
+ auto p = parser("requires readonly_and_readwrite_storage_textures,;");
+ p->requires_directive();
+ EXPECT_FALSE(p->has_error()) << p->error();
+
+ auto program = p->program();
+ auto& ast = program.AST();
+ ASSERT_EQ(ast.Requires().Length(), 1u);
+ auto* req = ast.Requires()[0];
+ EXPECT_EQ(req->source.range.begin.line, 1u);
+ EXPECT_EQ(req->source.range.begin.column, 1u);
+ EXPECT_EQ(req->source.range.end.line, 1u);
+ EXPECT_EQ(req->source.range.end.column, 51u);
+ ASSERT_EQ(req->features.Length(), 1u);
+ EXPECT_EQ(req->features[0], wgsl::LanguageFeature::kReadonlyAndReadwriteStorageTextures);
+ ASSERT_EQ(ast.GlobalDeclarations().Length(), 1u);
+ EXPECT_EQ(ast.GlobalDeclarations()[0], req);
+}
+
+TEST_F(RequiresDirectiveTest, Multiple_Repeated) {
+ auto p = parser(
+ "requires readonly_and_readwrite_storage_textures, "
+ "readonly_and_readwrite_storage_textures;");
+ p->requires_directive();
+ EXPECT_FALSE(p->has_error()) << p->error();
+
+ auto program = p->program();
+ auto& ast = program.AST();
+ ASSERT_EQ(ast.Requires().Length(), 1u);
+ auto* req = ast.Requires()[0];
+ EXPECT_EQ(req->source.range.begin.line, 1u);
+ EXPECT_EQ(req->source.range.begin.column, 1u);
+ EXPECT_EQ(req->source.range.end.line, 1u);
+ EXPECT_EQ(req->source.range.end.column, 91u);
+ ASSERT_EQ(req->features.Length(), 1u);
+ EXPECT_EQ(req->features[0], wgsl::LanguageFeature::kReadonlyAndReadwriteStorageTextures);
+ ASSERT_EQ(ast.GlobalDeclarations().Length(), 1u);
+ EXPECT_EQ(ast.GlobalDeclarations()[0], req);
}
// Test an unknown require identifier.
@@ -76,7 +129,7 @@
auto p = parser("requires;");
p->translation_unit();
EXPECT_TRUE(p->has_error());
- EXPECT_EQ(p->error(), R"(1:9: missing feature names in requires directive)");
+ EXPECT_EQ(p->error(), R"(1:9: invalid feature name for requires)");
}
}
