From 231648c6a9b418d9879afacedfd080d0aa2e9c58 Mon Sep 17 00:00:00 2001
From: Ben Clayton <bclayton@google.com>
Date: Fri, 27 Aug 2021 14:54:47 +0000
Subject: [PATCH] resolver: Validate storage class for var initializers

Bug: chromium:1243418
Change-Id: Ia0cec7d77767783b2a3b85400a03c805b51699d8
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/62942
Commit-Queue: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
---
 src/resolver/resolver.cc                | 35 ++++++++++++++++++++-----
 src/resolver/resolver.h                 |  1 +
 src/resolver/var_let_validation_test.cc | 12 +++++++++
 3 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index a172385ba7..b61cf818d4 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -520,15 +520,16 @@ Resolver::VariableInfo* Resolver::Variable(ast::Variable* var,
   }
 
   auto storage_class = var->declared_storage_class();
-  if (storage_class == ast::StorageClass::kNone) {
-    if (storage_type->UnwrapRef()->is_handle()) {
+  if (storage_class == ast::StorageClass::kNone && !var->is_const()) {
+    // No declared storage class. Infer from usage / type.
+    if (kind == VariableKind::kLocal) {
+      storage_class = ast::StorageClass::kFunction;
+    } else if (storage_type->UnwrapRef()->is_handle()) {
       // https://gpuweb.github.io/gpuweb/wgsl/#module-scope-variables
       // If the store type is a texture type or a sampler type, then the
       // variable declaration must not have a storage class decoration. The
       // storage class will always be handle.
       storage_class = ast::StorageClass::kUniformConstant;
-    } else if (kind == VariableKind::kLocal && !var->is_const()) {
-      storage_class = ast::StorageClass::kFunction;
     }
   }
 
@@ -545,8 +546,9 @@ Resolver::VariableInfo* Resolver::Variable(ast::Variable* var,
         builder_->create<sem::Reference>(storage_type, storage_class, access);
   }
 
-  if (rhs_type && !ValidateVariableConstructor(var, storage_type, type_name,
-                                               rhs_type, rhs_type_name)) {
+  if (rhs_type &&
+      !ValidateVariableConstructor(var, storage_class, storage_type, type_name,
+                                   rhs_type, rhs_type_name)) {
     return nullptr;
   }
 
@@ -573,6 +575,7 @@ ast::Access Resolver::DefaultAccessForStorageClass(
 }
 
 bool Resolver::ValidateVariableConstructor(const ast::Variable* var,
+                                           ast::StorageClass storage_class,
                                            const sem::Type* storage_type,
                                            const std::string& type_name,
                                            const sem::Type* rhs_type,
@@ -587,6 +590,26 @@ bool Resolver::ValidateVariableConstructor(const ast::Variable* var,
              var->source());
     return false;
   }
+
+  if (!var->is_const()) {
+    switch (storage_class) {
+      case ast::StorageClass::kPrivate:
+      case ast::StorageClass::kFunction:
+        break;  // Allowed an initializer
+      default:
+        // https://gpuweb.github.io/gpuweb/wgsl/#var-and-let
+        // Optionally has an initializer expression, if the variable is in the
+        // private or function storage classes.
+        AddError("var of storage class '" +
+                     std::string(ast::str(storage_class)) +
+                     "' cannot have an initializer. var initializers are only "
+                     "supported for the storage classes "
+                     "'private' and 'function'",
+                 var->source());
+        return false;
+    }
+  }
+
   return true;
 }
 
diff --git a/src/resolver/resolver.h b/src/resolver/resolver.h
index 90f305cb93..dbfe50679a 100644
--- a/src/resolver/resolver.h
+++ b/src/resolver/resolver.h
@@ -306,6 +306,7 @@ class Resolver {
   bool ValidateSwitch(const ast::SwitchStatement* s);
   bool ValidateVariable(const VariableInfo* info);
   bool ValidateVariableConstructor(const ast::Variable* var,
+                                   ast::StorageClass storage_class,
                                    const sem::Type* storage_type,
                                    const std::string& type_name,
                                    const sem::Type* rhs_type,
diff --git a/src/resolver/var_let_validation_test.cc b/src/resolver/var_let_validation_test.cc
index c549f5f11b..762ac007bb 100644
--- a/src/resolver/var_let_validation_test.cc
+++ b/src/resolver/var_let_validation_test.cc
@@ -341,6 +341,18 @@ TEST_F(ResolverVarLetValidationTest, NonConstructibleType_InferredType) {
             "12:34 error: function variable must have a constructible type");
 }
 
+TEST_F(ResolverVarLetValidationTest, InvalidStorageClassForInitializer) {
+  // var<workgroup> v : f32 = 1.23;
+  Global(Source{{12, 34}}, "v", ty.f32(), ast::StorageClass::kWorkgroup,
+         Expr(1.23f));
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(),
+            "12:34 error: var of storage class 'workgroup' cannot have "
+            "an initializer. var initializers are only supported for the "
+            "storage classes 'private' and 'function'");
+}
+
 }  // namespace
 }  // namespace resolver
 }  // namespace tint