[wgsl-reader] Remove explicit storage class from samplers/textures
Automatically set the storage class to UniformConstant.
Producing an error if an explicit storage class is given is deferred
until downstream users have caught up.
Bug: tint:332
Change-Id: I70e7390dc95d6f578a0fdeb675ca63a8b5b4fa26
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/40160
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Auto-Submit: James Price <jrprice@google.com>
diff --git a/src/ast/module_clone_test.cc b/src/ast/module_clone_test.cc
index dad6d56..a0c3211 100644
--- a/src/ast/module_clone_test.cc
+++ b/src/ast/module_clone_test.cc
@@ -46,11 +46,11 @@
var<uniform> g0 : u32 = 20u;
var<out> g1 : f32 = 123.0;
-var<uniform> g2 : texture_2d<f32>;
-var<uniform> g3 : [[access(read)]] texture_storage_2d<r32uint>;
-var<uniform> g4 : [[access(write)]] texture_storage_2d<rg32float>;
-var<uniform> g5 : [[access(read)]] texture_storage_2d<r32uint>;
-var<uniform> g6 : [[access(write)]] texture_storage_2d<rg32float>;
+var g2 : texture_2d<f32>;
+var g3 : [[access(read)]] texture_storage_2d<r32uint>;
+var g4 : [[access(write)]] texture_storage_2d<rg32float>;
+var g5 : [[access(read)]] texture_storage_2d<r32uint>;
+var g6 : [[access(write)]] texture_storage_2d<rg32float>;
[[builtin(position)]] var<uniform> g7 : vec3<f32>;
[[group(10), binding(20)]] var<storage> g7 : S;
diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc
index 07fb145..5c6440d 100644
--- a/src/reader/wgsl/parser_impl.cc
+++ b/src/reader/wgsl/parser_impl.cc
@@ -482,17 +482,24 @@
if (!match(Token::Type::kVar))
return Failure::kNoMatch;
- auto sc = variable_storage_decoration();
- if (sc.errored)
+ ast::StorageClass sc = ast::StorageClass::kNone;
+ auto explicit_sc = variable_storage_decoration();
+ if (explicit_sc.errored)
return Failure::kErrored;
+ if (explicit_sc.matched)
+ sc = explicit_sc.value;
auto decl = expect_variable_ident_decl("variable declaration");
if (decl.errored)
return Failure::kErrored;
- return VarDeclInfo{decl->source, decl->name,
- sc.matched ? sc.value : ast::StorageClass::kNone,
- decl->type};
+ if (decl->type->Is<type::Sampler>() || decl->type->Is<type::Texture>()) {
+ // sampler and texture variables implicitly have the storage class `handle`.
+ // TODO(jrprice): Produce an error if an explicit storage class is provided.
+ sc = ast::StorageClass::kUniformConstant;
+ }
+
+ return VarDeclInfo{decl->source, decl->name, sc, decl->type};
}
// texture_sampler_types
@@ -866,7 +873,7 @@
if (sc.errored)
return Failure::kErrored;
- return sc.value;
+ return sc;
}
// type_alias
@@ -1095,34 +1102,36 @@
// | FUNCTION
Expect<ast::StorageClass> ParserImpl::expect_storage_class(
const std::string& use) {
+ auto source = peek().source();
+
if (match(Token::Type::kIn))
- return ast::StorageClass::kInput;
+ return {ast::StorageClass::kInput, source};
if (match(Token::Type::kOut))
- return ast::StorageClass::kOutput;
+ return {ast::StorageClass::kOutput, source};
if (match(Token::Type::kUniform))
- return ast::StorageClass::kUniform;
+ return {ast::StorageClass::kUniform, source};
if (match(Token::Type::kWorkgroup))
- return ast::StorageClass::kWorkgroup;
+ return {ast::StorageClass::kWorkgroup, source};
if (match(Token::Type::kUniformConstant))
- return ast::StorageClass::kUniformConstant;
+ return {ast::StorageClass::kUniformConstant, source};
if (match(Token::Type::kStorage))
- return ast::StorageClass::kStorage;
+ return {ast::StorageClass::kStorage, source};
if (match(Token::Type::kImage))
- return ast::StorageClass::kImage;
+ return {ast::StorageClass::kImage, source};
if (match(Token::Type::kPrivate))
- return ast::StorageClass::kPrivate;
+ return {ast::StorageClass::kPrivate, source};
if (match(Token::Type::kFunction))
- return ast::StorageClass::kFunction;
+ return {ast::StorageClass::kFunction, source};
- return add_error(peek().source(), "invalid storage class", use);
+ return add_error(source, "invalid storage class", use);
}
// struct_decl
diff --git a/src/reader/wgsl/parser_impl_error_msg_test.cc b/src/reader/wgsl/parser_impl_error_msg_test.cc
index 24ac9f3..1a3020e 100644
--- a/src/reader/wgsl/parser_impl_error_msg_test.cc
+++ b/src/reader/wgsl/parser_impl_error_msg_test.cc
@@ -1083,6 +1083,24 @@
" ^\n");
}
+TEST_F(ParserImplErrorTest, DISABLED_GlobalDeclSamplerExplicitStorageClass) {
+ // TODO(jrprice): Enable this once downstream users have caught up.
+ EXPECT(
+ "var<uniform> x : sampler;",
+ "test.wgsl:1:5 error: sampler variables must not have a storage class\n"
+ "var<uniform> x : sampler;\n"
+ " ^^^^^^^\n");
+}
+
+TEST_F(ParserImplErrorTest, DISABLED_GlobalDeclTextureExplicitStorageClass) {
+ // TODO(jrprice): Enable this once downstream users have caught up.
+ EXPECT(
+ "var<uniform> x : texture_1d<f32>;",
+ "test.wgsl:1:5 error: texture variables must not have a storage class\n"
+ "var<uniform> x : texture_1d<f32>;\n"
+ " ^^^^^^^\n");
+}
+
TEST_F(ParserImplErrorTest, IfStmtMissingLParen) {
EXPECT("fn f() -> void { if true) {} }",
"test.wgsl:1:21 error: expected '('\n"
diff --git a/src/reader/wgsl/parser_impl_global_variable_decl_test.cc b/src/reader/wgsl/parser_impl_global_variable_decl_test.cc
index 625abb5..d23c642 100644
--- a/src/reader/wgsl/parser_impl_global_variable_decl_test.cc
+++ b/src/reader/wgsl/parser_impl_global_variable_decl_test.cc
@@ -174,6 +174,38 @@
EXPECT_EQ(p->error(), "1:5: invalid storage class for variable decoration");
}
+TEST_F(ParserImplTest, GlobalVariableDecl_SamplerImplicitStorageClass) {
+ auto p = parser("var s : sampler;");
+ auto decos = p->decoration_list();
+ EXPECT_FALSE(decos.errored);
+ EXPECT_FALSE(decos.matched);
+ auto e = p->global_variable_decl(decos.value);
+ ASSERT_FALSE(p->has_error()) << p->error();
+ EXPECT_FALSE(e.errored);
+ EXPECT_TRUE(e.matched);
+ ASSERT_NE(e.value, nullptr);
+
+ EXPECT_EQ(e->symbol(), p->builder().Symbols().Get("s"));
+ EXPECT_TRUE(e->type()->Is<type::Sampler>());
+ EXPECT_EQ(e->declared_storage_class(), ast::StorageClass::kUniformConstant);
+}
+
+TEST_F(ParserImplTest, GlobalVariableDecl_TextureImplicitStorageClass) {
+ auto p = parser("var s : texture_1d<f32>;");
+ auto decos = p->decoration_list();
+ EXPECT_FALSE(decos.errored);
+ EXPECT_FALSE(decos.matched);
+ auto e = p->global_variable_decl(decos.value);
+ ASSERT_FALSE(p->has_error()) << p->error();
+ EXPECT_FALSE(e.errored);
+ EXPECT_TRUE(e.matched);
+ ASSERT_NE(e.value, nullptr);
+
+ EXPECT_EQ(e->symbol(), p->builder().Symbols().Get("s"));
+ EXPECT_TRUE(e->type()->Is<type::Texture>());
+ EXPECT_EQ(e->declared_storage_class(), ast::StorageClass::kUniformConstant);
+}
+
} // namespace
} // namespace wgsl
} // namespace reader
