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

src/ast/module_clone_test.cc

@@ -46,11 +46,11 @@ const c1 : bool = true;
 
 var<uniform> g0 : u32 = 20u;
 var<out> g1 : f32 = 123.0;
-var<uniform> g2 : texture_2d<f32>;
+var g2 : texture_2d<f32>;
-var<uniform> g3 : [[access(read)]] texture_storage_2d<r32uint>;
+var g3 : [[access(read)]] texture_storage_2d<r32uint>;
-var<uniform> g4 : [[access(write)]] texture_storage_2d<rg32float>;
+var g4 : [[access(write)]] texture_storage_2d<rg32float>;
-var<uniform> g5 : [[access(read)]] texture_storage_2d<r32uint>;
+var g5 : [[access(read)]] texture_storage_2d<r32uint>;
-var<uniform> g6 : [[access(write)]] texture_storage_2d<rg32float>;
+var g6 : [[access(write)]] texture_storage_2d<rg32float>;
 
 [[builtin(position)]] var<uniform> g7 : vec3<f32>;
 [[group(10), binding(20)]] var<storage> g7 : S;

src/reader/wgsl/parser_impl.cc

@@ -482,17 +482,24 @@ Maybe<ParserImpl::VarDeclInfo> ParserImpl::variable_decl() {
   if (!match(Token::Type::kVar))
     return Failure::kNoMatch;
 
-  auto sc = variable_storage_decoration();
+  ast::StorageClass sc = ast::StorageClass::kNone;
-  if (sc.errored)
+  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,
+  if (decl->type->Is<type::Sampler>() || decl->type->Is<type::Texture>()) {
-                     sc.matched ? sc.value : ast::StorageClass::kNone,
+    // sampler and texture variables implicitly have the storage class `handle`.
-                     decl->type};
+    // 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 @@ Maybe<ast::StorageClass> ParserImpl::variable_storage_decoration() {
   if (sc.errored)
     return Failure::kErrored;
 
-  return sc.value;
+  return sc;
 }
 
 // type_alias
@@ -1095,34 +1102,36 @@ Expect<type::Type*> ParserImpl::expect_type_decl_matrix(Token t) {
 //  | 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

src/reader/wgsl/parser_impl_error_msg_test.cc

@@ -1083,6 +1083,24 @@ TEST_F(ParserImplErrorTest, GlobalDeclVarVectorMissingType) {
          "             ^\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"

src/reader/wgsl/parser_impl_global_variable_decl_test.cc

@@ -174,6 +174,38 @@ TEST_F(ParserImplTest, GlobalVariableDecl_InvalidVariableDecl) {
   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