tint: Improve module scope var diagnostic

Improve the error message when declaring a module-scope var without an address space, with an initializer.

Fixed: tint:1870
Change-Id: If087ae7dadb512c7050e89a0c75990080668e27d
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/123600
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>

src/tint/resolver/resolver.cc

@@ -260,7 +260,7 @@ sem::Variable* Resolver::Let(const ast::Let* v, bool is_global) {
         ty = rhs->Type()->UnwrapRef();  // Implicit load of RHS
     }
 
-    if (rhs && !validator_.VariableInitializer(v, builtin::AddressSpace::kUndefined, ty, rhs)) {
+    if (rhs && !validator_.VariableInitializer(v, ty, rhs)) {
         return nullptr;
     }
 
@@ -323,7 +323,7 @@ sem::Variable* Resolver::Override(const ast::Override* v) {
         return nullptr;
     }
 
-    if (rhs && !validator_.VariableInitializer(v, builtin::AddressSpace::kUndefined, ty, rhs)) {
+    if (rhs && !validator_.VariableInitializer(v, ty, rhs)) {
         return nullptr;
     }
 
@@ -417,7 +417,7 @@ sem::Variable* Resolver::Const(const ast::Const* c, bool is_global) {
         ty = rhs->Type();
     }
 
-    if (!validator_.VariableInitializer(c, builtin::AddressSpace::kUndefined, ty, rhs)) {
+    if (!validator_.VariableInitializer(c, ty, rhs)) {
         return nullptr;
     }
 
@@ -516,7 +516,7 @@ sem::Variable* Resolver::Var(const ast::Var* var, bool is_global) {
         access = DefaultAccessForAddressSpace(address_space);
     }
 
-    if (rhs && !validator_.VariableInitializer(var, address_space, storage_ty, rhs)) {
+    if (rhs && !validator_.VariableInitializer(var, storage_ty, rhs)) {
         return nullptr;
     }
 

src/tint/resolver/validator.cc

@@ -373,7 +373,6 @@ bool Validator::Materialize(const type::Type* to,
 }
 
 bool Validator::VariableInitializer(const ast::Variable* v,
-                                    builtin::AddressSpace address_space,
                                     const type::Type* storage_ty,
                                     const sem::ValueExpression* initializer) const {
     auto* initializer_ty = initializer->Type();
@@ -388,23 +387,6 @@ bool Validator::VariableInitializer(const ast::Variable* v,
         return false;
     }
 
-    if (v->Is<ast::Var>()) {
-        switch (address_space) {
-            case builtin::AddressSpace::kPrivate:
-            case builtin::AddressSpace::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 address spaces.
-                AddError("var of address space '" + utils::ToString(address_space) +
-                             "' cannot have an initializer. var initializers are only supported "
-                             "for the address spaces 'private' and 'function'",
-                         v->source);
-                return false;
-        }
-    }
-
     return true;
 }
 
@@ -728,6 +710,23 @@ bool Validator::Var(const sem::Variable* v) const {
         }
     }
 
+    if (var->initializer) {
+        switch (v->AddressSpace()) {
+            case builtin::AddressSpace::kPrivate:
+            case builtin::AddressSpace::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 address spaces.
+                AddError("var of address space '" + utils::ToString(v->AddressSpace()) +
+                             "' cannot have an initializer. var initializers are only supported "
+                             "for the address spaces 'private' and 'function'",
+                         var->source);
+                return false;
+        }
+    }
+
     if (!CheckTypeAccessAddressSpace(v->Type()->UnwrapRef(), v->Access(), v->AddressSpace(),
                                      var->attributes, var->source)) {
         return false;

src/tint/resolver/validator.h

@@ -429,12 +429,10 @@ class Validator {
 
     /// Validates a variable initializer
     /// @param v the variable to validate
-    /// @param address_space the address space of the variable
     /// @param storage_type the type of the storage
     /// @param initializer the RHS initializer expression
     /// @returns true on succes, false otherwise
     bool VariableInitializer(const ast::Variable* v,
-                             builtin::AddressSpace address_space,
                              const type::Type* storage_type,
                              const sem::ValueExpression* initializer) const;
 

src/tint/resolver/variable_validation_test.cc

@@ -58,6 +58,26 @@ TEST_F(ResolverVariableValidationTest, GlobalVarInitializerNoReturnValueBuiltin)
     EXPECT_EQ(r()->error(), "12:34 error: builtin 'storageBarrier' does not return a value");
 }
 
+TEST_F(ResolverVariableValidationTest, GlobalVarNoAddressSpace) {
+    // var a : i32;
+    GlobalVar(Source{{12, 34}}, "a", ty.i32());
+
+    EXPECT_FALSE(r()->Resolve());
+    EXPECT_EQ(
+        r()->error(),
+        R"(12:34 error: module-scope 'var' declarations that are not of texture or sampler types must provide an address space)");
+}
+
+TEST_F(ResolverVariableValidationTest, GlobalVarWithInitializerNoAddressSpace) {
+    // var a = 1;
+    GlobalVar(Source{{12, 34}}, "a", Expr(1_a));
+
+    EXPECT_FALSE(r()->Resolve());
+    EXPECT_EQ(
+        r()->error(),
+        R"(12:34 error: module-scope 'var' declarations that are not of texture or sampler types must provide an address space)");
+}
+
 TEST_F(ResolverVariableValidationTest, GlobalVarUsedAtModuleScope) {
     // var<private> a : i32;
     // var<private> b : i32 = a;