commit 61306b882debd853b65999d564878eaa3a4c8a8a
author James Price <jrprice@google.com> Mon Feb 08 15:08:49 2021 +0000
committer Commit Bot service account <commit-bot@chromium.org> Mon Feb 08 15:08:49 2021 +0000
tree e399b0decaa59d52695898100b26b327d65b9fca
parent 6d1687fb01de9fbe16982a39bb818461f3367ec1

[wgsl-reader][wgsl-writer] Fix implicit handle storage class for textures

Unwrap the type before checking if it is a handle type, to account for
access decorations.

Bug: tint:332
Change-Id: I8af9749fec1e2f5dbd7c3bec0b73e506ae111a28
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/40540
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>

src/reader/wgsl/parser_impl.cc

diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc
index 3bbf111..63f15c6 100644
--- a/src/reader/wgsl/parser_impl.cc
+++ b/src/reader/wgsl/parser_impl.cc
@@ -493,7 +493,7 @@
   if (decl.errored)
     return Failure::kErrored;
 
-  if (decl->type->is_handle()) {
+  if (decl->type->UnwrapAll()->is_handle()) {
     // handle types implicitly have the `UniformConstant` storage class.
     // TODO(jrprice): Produce an error if an explicit storage class is provided.
     sc = ast::StorageClass::kUniformConstant;

src/reader/wgsl/parser_impl_error_msg_test.cc

diff --git a/src/reader/wgsl/parser_impl_error_msg_test.cc b/src/reader/wgsl/parser_impl_error_msg_test.cc
index 1a3020e..eec3330 100644
--- a/src/reader/wgsl/parser_impl_error_msg_test.cc
+++ b/src/reader/wgsl/parser_impl_error_msg_test.cc
@@ -1095,9 +1095,9 @@
 TEST_F(ParserImplErrorTest, DISABLED_GlobalDeclTextureExplicitStorageClass) {
   // TODO(jrprice): Enable this once downstream users have caught up.
   EXPECT(
-      "var<uniform> x : texture_1d<f32>;",
+      "var<uniform> x : [[access(read)]] texture_1d<f32>;",
       "test.wgsl:1:5 error: texture variables must not have a storage class\n"
-      "var<uniform> x : texture_1d<f32>;\n"
+      "var<uniform> x : [[access(read)]] texture_1d<f32>;\n"
       "    ^^^^^^^\n");
 }
 

src/reader/wgsl/parser_impl_global_variable_decl_test.cc

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 d23c642..941d043 100644
--- a/src/reader/wgsl/parser_impl_global_variable_decl_test.cc
+++ b/src/reader/wgsl/parser_impl_global_variable_decl_test.cc
@@ -191,7 +191,7 @@
 }
 
 TEST_F(ParserImplTest, GlobalVariableDecl_TextureImplicitStorageClass) {
-  auto p = parser("var s : texture_1d<f32>;");
+  auto p = parser("var s : [[access(read)]] texture_1d<f32>;");
   auto decos = p->decoration_list();
   EXPECT_FALSE(decos.errored);
   EXPECT_FALSE(decos.matched);
@@ -202,7 +202,7 @@
   ASSERT_NE(e.value, nullptr);
 
   EXPECT_EQ(e->symbol(), p->builder().Symbols().Get("s"));
-  EXPECT_TRUE(e->type()->Is<type::Texture>());
+  EXPECT_TRUE(e->type()->UnwrapAll()->Is<type::Texture>());
   EXPECT_EQ(e->declared_storage_class(), ast::StorageClass::kUniformConstant);
 }
 

src/writer/wgsl/generator_impl.cc

diff --git a/src/writer/wgsl/generator_impl.cc b/src/writer/wgsl/generator_impl.cc
index a031363..4e08026 100644
--- a/src/writer/wgsl/generator_impl.cc
+++ b/src/writer/wgsl/generator_impl.cc
@@ -598,7 +598,7 @@
     out_ << "var";
     if (sem->StorageClass() != ast::StorageClass::kNone &&
         sem->StorageClass() != ast::StorageClass::kFunction &&
-        !var->type()->is_handle()) {
+        !var->type()->UnwrapAll()->is_handle()) {
       out_ << "<" << sem->StorageClass() << ">";
     }
   }

src/writer/wgsl/generator_impl_variable_decl_statement_test.cc

diff --git a/src/writer/wgsl/generator_impl_variable_decl_statement_test.cc b/src/writer/wgsl/generator_impl_variable_decl_statement_test.cc
index 633bff9..3290824 100644
--- a/src/writer/wgsl/generator_impl_variable_decl_statement_test.cc
+++ b/src/writer/wgsl/generator_impl_variable_decl_statement_test.cc
@@ -19,6 +19,7 @@
 #include "src/ast/identifier_expression.h"
 #include "src/ast/variable.h"
 #include "src/ast/variable_decl_statement.h"
+#include "src/type/access_control_type.h"
 #include "src/type/f32_type.h"
 #include "src/type/sampled_texture_type.h"
 #include "src/writer/wgsl/generator_impl.h"
@@ -92,9 +93,11 @@
 }
 
 TEST_F(WgslGeneratorImplTest, Emit_VariableDeclStatement_Texture) {
-  auto* var = Global(
-      "t", ast::StorageClass::kUniformConstant,
-      create<type::SampledTexture>(type::TextureDimension::k1d, ty.f32()));
+  auto* st =
+      create<type::SampledTexture>(type::TextureDimension::k1d, ty.f32());
+  auto* var =
+      Global("t", ast::StorageClass::kUniformConstant,
+             create<type::AccessControl>(ast::AccessControl::kReadOnly, st));
 
   auto* stmt = create<ast::VariableDeclStatement>(var);
 
@@ -103,7 +106,7 @@
   gen.increment_indent();
 
   ASSERT_TRUE(gen.EmitStatement(stmt)) << gen.error();
-  EXPECT_EQ(gen.result(), "  var t : texture_1d<f32>;\n");
+  EXPECT_EQ(gen.result(), "  var t : [[access(read)]]\ntexture_1d<f32>;\n");
 }
 
 }  // namespace