From 1f2d38c826ce25bf9d7f67453538f7467cf27541 Mon Sep 17 00:00:00 2001
From: James Price <jrprice@google.com>
Date: Thu, 4 Feb 2021 18:35:40 +0000
Subject: [PATCH] [wgsl-writer] Omit the storage class for handle types

These types have an implicit storage class of UniformConstant.

Bug: tint:332
Change-Id: I78c5b2a085e543ebba7d5e92b8f11550d0cd4d49
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/40400
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: James Price <jrprice@google.com>
---
 src/reader/wgsl/parser_impl.cc                |  4 +--
 src/type/type.cc                              |  4 +++
 src/type/type.h                               |  2 ++
 src/writer/wgsl/generator_impl.cc             |  3 +-
 ...rator_impl_variable_decl_statement_test.cc | 30 +++++++++++++++++++
 5 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc
index 5c6440d43d..3bbf1110f7 100644
--- a/src/reader/wgsl/parser_impl.cc
+++ b/src/reader/wgsl/parser_impl.cc
@@ -493,8 +493,8 @@ Maybe<ParserImpl::VarDeclInfo> ParserImpl::variable_decl() {
   if (decl.errored)
     return Failure::kErrored;
 
-  if (decl->type->Is<type::Sampler>() || decl->type->Is<type::Texture>()) {
-    // sampler and texture variables implicitly have the storage class `handle`.
+  if (decl->type->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;
   }
diff --git a/src/type/type.cc b/src/type/type.cc
index 29f95e2362..679e4e3d42 100644
--- a/src/type/type.cc
+++ b/src/type/type.cc
@@ -127,5 +127,9 @@ bool Type::is_bool_scalar_or_vector() const {
   return Is<Bool>() || is_bool_vector();
 }
 
+bool Type::is_handle() const {
+  return Is<type::Sampler>() || Is<type::Texture>();
+}
+
 }  // namespace type
 }  // namespace tint
diff --git a/src/type/type.h b/src/type/type.h
index 0d6dc04303..2834e7bfc1 100644
--- a/src/type/type.h
+++ b/src/type/type.h
@@ -99,6 +99,8 @@ class Type : public Castable<Type> {
   bool is_bool_vector() const;
   /// @returns true if this type is boolean scalar or vector
   bool is_bool_scalar_or_vector() const;
+  /// @returns true if this type is a handle type
+  bool is_handle() const;
 
  protected:
   Type();
diff --git a/src/writer/wgsl/generator_impl.cc b/src/writer/wgsl/generator_impl.cc
index 3025ffddc9..a031363cd1 100644
--- a/src/writer/wgsl/generator_impl.cc
+++ b/src/writer/wgsl/generator_impl.cc
@@ -597,7 +597,8 @@ bool GeneratorImpl::EmitVariable(ast::Variable* var) {
   } else {
     out_ << "var";
     if (sem->StorageClass() != ast::StorageClass::kNone &&
-        sem->StorageClass() != ast::StorageClass::kFunction) {
+        sem->StorageClass() != ast::StorageClass::kFunction &&
+        !var->type()->is_handle()) {
       out_ << "<" << sem->StorageClass() << ">";
     }
   }
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 b890b0a49b..633bff94c0 100644
--- a/src/writer/wgsl/generator_impl_variable_decl_statement_test.cc
+++ b/src/writer/wgsl/generator_impl_variable_decl_statement_test.cc
@@ -20,6 +20,7 @@
 #include "src/ast/variable.h"
 #include "src/ast/variable_decl_statement.h"
 #include "src/type/f32_type.h"
+#include "src/type/sampled_texture_type.h"
 #include "src/writer/wgsl/generator_impl.h"
 #include "src/writer/wgsl/test_helper.h"
 
@@ -76,6 +77,35 @@ TEST_F(WgslGeneratorImplTest, Emit_VariableDeclStatement_Private) {
   EXPECT_EQ(gen.result(), "  var<private> a : f32;\n");
 }
 
+TEST_F(WgslGeneratorImplTest, Emit_VariableDeclStatement_Sampler) {
+  auto* var = Global("s", ast::StorageClass::kUniformConstant,
+                     create<type::Sampler>(type::SamplerKind::kSampler));
+
+  auto* stmt = create<ast::VariableDeclStatement>(var);
+
+  GeneratorImpl& gen = Build();
+
+  gen.increment_indent();
+
+  ASSERT_TRUE(gen.EmitStatement(stmt)) << gen.error();
+  EXPECT_EQ(gen.result(), "  var s : sampler;\n");
+}
+
+TEST_F(WgslGeneratorImplTest, Emit_VariableDeclStatement_Texture) {
+  auto* var = Global(
+      "t", ast::StorageClass::kUniformConstant,
+      create<type::SampledTexture>(type::TextureDimension::k1d, ty.f32()));
+
+  auto* stmt = create<ast::VariableDeclStatement>(var);
+
+  GeneratorImpl& gen = Build();
+
+  gen.increment_indent();
+
+  ASSERT_TRUE(gen.EmitStatement(stmt)) << gen.error();
+  EXPECT_EQ(gen.result(), "  var t : texture_1d<f32>;\n");
+}
+
 }  // namespace
 }  // namespace wgsl
 }  // namespace writer