tint/resolver: Clean up 'var' resolving

Split the logic out of Resolver::Variable().

Also correctly allows type inference of module-scoped 'var's.

Fixed: tint:1584
Change-Id: I32eb11f0a847775137fef937da6f4032a3b3c2b9
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/93784
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>

src/tint/resolver/atomics_validation_test.cc

@@ -309,7 +309,7 @@ TEST_F(ResolverAtomicValidationTest, Local) {
     WrapInFunction(Var("a", ty.atomic(Source{{12, 34}}, ty.i32())));
 
     EXPECT_FALSE(r()->Resolve());
-    EXPECT_EQ(r()->error(), "12:34 error: function variable must have a constructible type");
+    EXPECT_EQ(r()->error(), "12:34 error: function-scope 'var' must have a constructible type");
 }
 
 }  // namespace

src/tint/resolver/builtin_validation_test.cc

@@ -90,7 +90,7 @@ TEST_F(ResolverBuiltinValidationTest, BuiltinRedeclaredAsGlobalLet) {
 
     EXPECT_FALSE(r()->Resolve());
     EXPECT_EQ(r()->error(),
-              R"(12:34 error: 'mix' is a builtin and cannot be redeclared as a module-scope let)");
+              R"(12:34 error: 'mix' is a builtin and cannot be redeclared as a 'let')");
 }
 
 TEST_F(ResolverBuiltinValidationTest, BuiltinRedeclaredAsGlobalVar) {
@@ -98,7 +98,7 @@ TEST_F(ResolverBuiltinValidationTest, BuiltinRedeclaredAsGlobalVar) {
 
     EXPECT_FALSE(r()->Resolve());
     EXPECT_EQ(r()->error(),
-              R"(12:34 error: 'mix' is a builtin and cannot be redeclared as a module-scope var)");
+              R"(12:34 error: 'mix' is a builtin and cannot be redeclared as a module-scope 'var')");
 }
 
 TEST_F(ResolverBuiltinValidationTest, BuiltinRedeclaredAsAlias) {

src/tint/resolver/host_shareable_validation_test.cc

@@ -38,7 +38,7 @@ TEST_F(ResolverHostShareableValidationTest, BoolMember) {
         r()->error(),
         R"(56:78 error: Type 'bool' cannot be used in storage class 'storage' as it is non-host-shareable
 12:34 note: while analysing structure member S.x
-56:78 note: while instantiating variable g)");
+56:78 note: while instantiating 'var' g)");
 }
 
 TEST_F(ResolverHostShareableValidationTest, BoolVectorMember) {
@@ -56,7 +56,7 @@ TEST_F(ResolverHostShareableValidationTest, BoolVectorMember) {
         r()->error(),
         R"(56:78 error: Type 'vec3<bool>' cannot be used in storage class 'storage' as it is non-host-shareable
 12:34 note: while analysing structure member S.x
-56:78 note: while instantiating variable g)");
+56:78 note: while instantiating 'var' g)");
 }
 
 TEST_F(ResolverHostShareableValidationTest, Aliases) {
@@ -75,7 +75,7 @@ TEST_F(ResolverHostShareableValidationTest, Aliases) {
         r()->error(),
         R"(56:78 error: Type 'bool' cannot be used in storage class 'storage' as it is non-host-shareable
 12:34 note: while analysing structure member S.x
-56:78 note: while instantiating variable g)");
+56:78 note: while instantiating 'var' g)");
 }
 
 TEST_F(ResolverHostShareableValidationTest, NestedStructures) {
@@ -100,7 +100,7 @@ TEST_F(ResolverHostShareableValidationTest, NestedStructures) {
 3:4 note: while analysing structure member I2.y
 5:6 note: while analysing structure member I3.z
 7:8 note: while analysing structure member S.m
-9:10 note: while instantiating variable g)");
+9:10 note: while instantiating 'var' g)");
 }
 
 TEST_F(ResolverHostShareableValidationTest, NoError) {

src/tint/resolver/inferred_type_test.cc

@@ -89,16 +89,18 @@ TEST_P(ResolverInferredTypeParamTest, GlobalLet_Pass) {
     EXPECT_EQ(TypeOf(var), expected_type);
 }
 
-TEST_P(ResolverInferredTypeParamTest, GlobalVar_Fail) {
+TEST_P(ResolverInferredTypeParamTest, GlobalVar_Pass) {
     auto& params = GetParam();
 
+    auto* expected_type = params.create_expected_type(*this);
+
     // var a = <type constructor>;
     auto* ctor_expr = params.create_value(*this, 0);
-    Global(Source{{12, 34}}, "a", nullptr, ast::StorageClass::kPrivate, ctor_expr);
+    auto* var = Global("a", nullptr, ast::StorageClass::kPrivate, ctor_expr);
     WrapInFunction();
 
-    EXPECT_FALSE(r()->Resolve());
+    EXPECT_TRUE(r()->Resolve()) << r()->error();
-    EXPECT_EQ(r()->error(), "12:34 error: module-scope 'var' declaration must specify a type");
+    EXPECT_EQ(TypeOf(var)->UnwrapRef(), expected_type);
 }
 
 TEST_P(ResolverInferredTypeParamTest, LocalLet_Pass) {

src/tint/resolver/resolver.cc

@@ -312,17 +312,16 @@ sem::Type* Resolver::Type(const ast::Type* ty) {
 }
 
 sem::Variable* Resolver::Variable(const ast::Variable* v, bool is_global) {
-    const sem::Type* storage_ty = nullptr;
+    const sem::Type* ty = nullptr;
 
     // If the variable has a declared type, resolve it.
-    if (auto* ty = v->type) {
+    if (v->type) {
-        storage_ty = Type(ty);
+        ty = Type(v->type);
-        if (!storage_ty) {
+        if (!ty) {
             return nullptr;
         }
     }
 
-    auto* as_var = v->As<ast::Var>();
     auto* as_let = v->As<ast::Let>();
     auto* as_override = v->As<ast::Override>();
 
@@ -330,39 +329,91 @@ sem::Variable* Resolver::Variable(const ast::Variable* v, bool is_global) {
 
     // Does the variable have a constructor?
     if (v->constructor) {
-        rhs = Materialize(Expression(v->constructor), storage_ty);
+        rhs = Materialize(Expression(v->constructor), ty);
         if (!rhs) {
             return nullptr;
         }
 
         // If the variable has no declared type, infer it from the RHS
-        if (!storage_ty) {
+        if (!ty) {
-            if (as_var && is_global) {
+            ty = rhs->Type()->UnwrapRef();  // Implicit load of RHS
-                AddError("module-scope 'var' declaration must specify a type", v->source);
-                return nullptr;
-            }
-
-            storage_ty = rhs->Type()->UnwrapRef();  // Implicit load of RHS
         }
     } else if (as_let) {
         AddError("'let' declaration must have an initializer", v->source);
         return nullptr;
-    } else if (!v->type) {
+    } else if (!ty) {
-        AddError((is_global) ? "module-scope 'var' declaration requires a type or initializer"
+        AddError("'override' declaration requires a type or initializer", v->source);
-                             : "function-scope 'var' declaration requires a type or initializer",
-                 v->source);
         return nullptr;
     }
 
+    if (rhs &&
+        !validator_.VariableConstructorOrCast(v, ast::StorageClass::kNone, ty, rhs->Type())) {
+        return nullptr;
+    }
+
+    if (!ApplyStorageClassUsageToType(ast::StorageClass::kNone, const_cast<sem::Type*>(ty),
+                                      v->source)) {
+        AddNote("while instantiating variable " + builder_->Symbols().NameFor(v->symbol),
+                v->source);
+        return nullptr;
+    }
+
+    sem::Variable* sem = nullptr;
+    if (is_global) {
+        bool has_const_val = rhs && !as_override;
+        auto* global = builder_->create<sem::GlobalVariable>(
+            v, ty, ast::StorageClass::kNone, ast::Access::kUndefined,
+            has_const_val ? rhs->ConstantValue() : sem::Constant{}, sem::BindingPoint{});
+
+        if (as_override) {
+            if (auto* id = ast::GetAttribute<ast::IdAttribute>(v->attributes)) {
+                global->SetConstantId(static_cast<uint16_t>(id->value));
+            }
+        }
+        sem = global;
+    } else {
+        sem = builder_->create<sem::LocalVariable>(
+            v, ty, ast::StorageClass::kNone, ast::Access::kUndefined, current_statement_,
+            (rhs && as_let) ? rhs->ConstantValue() : sem::Constant{});
+    }
+
+    sem->SetConstructor(rhs);
+    builder_->Sem().Add(v, sem);
+    return sem;
+}
+
+sem::Variable* Resolver::Var(const ast::Var* var, bool is_global) {
+    const sem::Type* storage_ty = nullptr;
+
+    // If the variable has a declared type, resolve it.
+    if (auto* ty = var->type) {
+        storage_ty = Type(ty);
+        if (!storage_ty) {
+            return nullptr;
+        }
+    }
+
+    const sem::Expression* rhs = nullptr;
+
+    // Does the variable have a constructor?
+    if (var->constructor) {
+        rhs = Materialize(Expression(var->constructor), storage_ty);
+        if (!rhs) {
+            return nullptr;
+        }
+        // If the variable has no declared type, infer it from the RHS
+        if (!storage_ty) {
+            storage_ty = rhs->Type()->UnwrapRef();  // Implicit load of RHS
+        }
+    }
+
     if (!storage_ty) {
-        TINT_ICE(Resolver, diagnostics_) << "failed to determine storage type for variable '" +
+        AddError("'var' declaration requires a type or initializer", var->source);
-                                                builder_->Symbols().NameFor(v->symbol) + "'\n"
-                                         << "Source: " << v->source;
         return nullptr;
     }
 
-    auto storage_class = as_var ? as_var->declared_storage_class : ast::StorageClass::kNone;
+    auto storage_class = var->declared_storage_class;
-    if (storage_class == ast::StorageClass::kNone && as_var) {
+    if (storage_class == ast::StorageClass::kNone) {
         // No declared storage class. Infer from usage / type.
         if (!is_global) {
             storage_class = ast::StorageClass::kFunction;
@@ -375,65 +426,47 @@ sem::Variable* Resolver::Variable(const ast::Variable* v, bool is_global) {
         }
     }
 
-    if (!is_global && as_var && storage_class != ast::StorageClass::kFunction &&
+    if (!is_global && storage_class != ast::StorageClass::kFunction &&
-        validator_.IsValidationEnabled(v->attributes,
+        validator_.IsValidationEnabled(var->attributes,
                                        ast::DisabledValidation::kIgnoreStorageClass)) {
-        AddError("function-scope 'var' declaration must use 'function' storage class", v->source);
+        AddError("function-scope 'var' declaration must use 'function' storage class", var->source);
         return nullptr;
     }
 
-    auto access = as_var ? as_var->declared_access : ast::Access::kUndefined;
+    auto access = var->declared_access;
     if (access == ast::Access::kUndefined) {
         access = DefaultAccessForStorageClass(storage_class);
     }
 
-    auto* var_ty = storage_ty;
+    if (rhs && !validator_.VariableConstructorOrCast(var, storage_class, storage_ty, rhs->Type())) {
-    if (as_var) {
-        // Variable declaration. Unlike `let` and parameters, `var` has storage.
-        // Variables are always of a reference type to the declared storage type.
-        var_ty = builder_->create<sem::Reference>(storage_ty, storage_class, access);
-    }
-
-    if (rhs && !validator_.VariableConstructorOrCast(v, storage_class, storage_ty, rhs->Type())) {
         return nullptr;
     }
 
-    if (!ApplyStorageClassUsageToType(storage_class, const_cast<sem::Type*>(var_ty), v->source)) {
+    auto* var_ty = builder_->create<sem::Reference>(storage_ty, storage_class, access);
-        AddNote("while instantiating variable " + builder_->Symbols().NameFor(v->symbol),
+
-                v->source);
+    if (!ApplyStorageClassUsageToType(storage_class, var_ty, var->source)) {
+        AddNote("while instantiating 'var' " + builder_->Symbols().NameFor(var->symbol),
+                var->source);
         return nullptr;
     }
 
+    sem::Variable* sem = nullptr;
     if (is_global) {
         sem::BindingPoint binding_point;
-        if (as_var) {
+        if (auto bp = var->BindingPoint()) {
-            if (auto bp = as_var->BindingPoint()) {
+            binding_point = {bp.group->value, bp.binding->value};
-                binding_point = {bp.group->value, bp.binding->value};
-            }
         }
+        sem = builder_->create<sem::GlobalVariable>(var, var_ty, storage_class, access,
+                                                    sem::Constant{}, binding_point);
 
-        bool has_const_val = rhs && as_let && !as_override;
+    } else {
-        auto* global = builder_->create<sem::GlobalVariable>(
+        sem = builder_->create<sem::LocalVariable>(var, var_ty, storage_class, access,
-            v, var_ty, storage_class, access,
+                                                   current_statement_, sem::Constant{});
-            has_const_val ? rhs->ConstantValue() : sem::Constant{}, binding_point);
-
-        if (as_override) {
-            if (auto* id = ast::GetAttribute<ast::IdAttribute>(v->attributes)) {
-                global->SetConstantId(static_cast<uint16_t>(id->value));
-            }
-        }
-
-        global->SetConstructor(rhs);
-        builder_->Sem().Add(v, global);
-        return global;
     }
 
-    auto* local = builder_->create<sem::LocalVariable>(
+    sem->SetConstructor(rhs);
-        v, var_ty, storage_class, access, current_statement_,
+    builder_->Sem().Add(var, sem);
-        (rhs && as_let) ? rhs->ConstantValue() : sem::Constant{});
+    return sem;
-    builder_->Sem().Add(v, local);
-    local->SetConstructor(rhs);
-    return local;
 }
 
 sem::Parameter* Resolver::Parameter(const ast::Parameter* param, uint32_t index) {
@@ -534,19 +567,14 @@ void Resolver::SetShadows() {
 }
 
 sem::GlobalVariable* Resolver::GlobalVariable(const ast::Variable* v) {
-    auto* sem = As<sem::GlobalVariable>(Variable(v, /* is_global */ true));
+    auto* as_var = v->As<ast::Var>();
+
+    auto* sem = As<sem::GlobalVariable>(as_var ? Var(as_var, /* is_global */ true)
+                                               : Variable(v, /* is_global */ true));
     if (!sem) {
         return nullptr;
     }
 
-    const bool is_var = v->Is<ast::Var>();
-
-    auto storage_class = sem->StorageClass();
-    if (is_var && storage_class == ast::StorageClass::kNone) {
-        AddError("module-scope 'var' declaration must have a storage class", v->source);
-        return nullptr;
-    }
-
     for (auto* attr : v->attributes) {
         Mark(attr);
 
@@ -570,7 +598,7 @@ sem::GlobalVariable* Resolver::GlobalVariable(const ast::Variable* v) {
         return nullptr;
     }
 
-    return sem->As<sem::GlobalVariable>();
+    return sem;
 }
 
 sem::Function* Resolver::Function(const ast::Function* decl) {
@@ -2449,8 +2477,11 @@ sem::Statement* Resolver::VariableDeclStatement(const ast::VariableDeclStatement
     return StatementScope(stmt, sem, [&] {
         Mark(stmt->variable);
 
-        auto* var = Variable(stmt->variable, /* is_global */ false);
+        auto* variable = Switch(
-        if (!var) {
+            stmt->variable,  //
+            [&](const ast::Var* var) { return Var(var, /* is_global */ false); },
+            [&](Default) { return Variable(stmt->variable, /* is_global */ false); });
+        if (!variable) {
             return false;
         }
 
@@ -2466,11 +2497,14 @@ sem::Statement* Resolver::VariableDeclStatement(const ast::VariableDeclStatement
             current_block_->AddDecl(stmt->variable);
         }
 
-        if (auto* ctor = var->Constructor()) {
+        if (auto* ctor = variable->Constructor()) {
             sem->Behaviors() = ctor->Behaviors();
         }
 
-        return validator_.Variable(var);
+        return Switch(
+            stmt->variable,  //
+            [&](const ast::Var*) { return validator_.Var(variable); },
+            [&](Default) { return validator_.Variable(variable); });
     });
 }
 

src/tint/resolver/resolver.h

@@ -298,6 +298,14 @@ class Resolver {
     /// @param is_global true if this is module scope, otherwise function scope
     sem::Variable* Variable(const ast::Variable* var, bool is_global);
 
+    /// @returns the semantic info for the `ast::Var` `var`. If an error is raised, nullptr is
+    /// returned.
+    /// @note this method does not resolve the attributes as these are context-dependent (global,
+    /// local)
+    /// @param var the variable
+    /// @param is_global true if this is module scope, otherwise function scope
+    sem::Variable* Var(const ast::Var* var, bool is_global);
+
     /// @returns the semantic info for the function parameter `param`. If an error is raised,
     /// nullptr is returned.
     /// @note the caller is expected to validate the parameter

src/tint/resolver/storage_class_validation_test.cc

@@ -49,7 +49,7 @@ TEST_F(ResolverStorageClassValidationTest, Private_RuntimeArray) {
     EXPECT_FALSE(r()->Resolve());
     EXPECT_EQ(r()->error(),
               R"(12:34 error: runtime-sized arrays can only be used in the <storage> storage class
-12:34 note: while instantiating variable v)");
+12:34 note: while instantiating 'var' v)");
 }
 
 TEST_F(ResolverStorageClassValidationTest, Private_RuntimeArrayInStruct) {
@@ -60,7 +60,7 @@ TEST_F(ResolverStorageClassValidationTest, Private_RuntimeArrayInStruct) {
     EXPECT_EQ(r()->error(),
               R"(12:34 error: runtime-sized arrays can only be used in the <storage> storage class
 note: while analysing structure member S.m
-12:34 note: while instantiating variable v)");
+12:34 note: while instantiating 'var' v)");
 }
 
 TEST_F(ResolverStorageClassValidationTest, Workgroup_RuntimeArray) {
@@ -69,7 +69,7 @@ TEST_F(ResolverStorageClassValidationTest, Workgroup_RuntimeArray) {
     EXPECT_FALSE(r()->Resolve());
     EXPECT_EQ(r()->error(),
               R"(12:34 error: runtime-sized arrays can only be used in the <storage> storage class
-12:34 note: while instantiating variable v)");
+12:34 note: while instantiating 'var' v)");
 }
 
 TEST_F(ResolverStorageClassValidationTest, Workgroup_RuntimeArrayInStruct) {
@@ -80,7 +80,7 @@ TEST_F(ResolverStorageClassValidationTest, Workgroup_RuntimeArrayInStruct) {
     EXPECT_EQ(r()->error(),
               R"(12:34 error: runtime-sized arrays can only be used in the <storage> storage class
 note: while analysing structure member S.m
-12:34 note: while instantiating variable v)");
+12:34 note: while instantiating 'var' v)");
 }
 
 TEST_F(ResolverStorageClassValidationTest, StorageBufferBool) {
@@ -96,7 +96,7 @@ TEST_F(ResolverStorageClassValidationTest, StorageBufferBool) {
     EXPECT_EQ(
         r()->error(),
         R"(56:78 error: Type 'bool' cannot be used in storage class 'storage' as it is non-host-shareable
-56:78 note: while instantiating variable g)");
+56:78 note: while instantiating 'var' g)");
 }
 
 TEST_F(ResolverStorageClassValidationTest, StorageBufferPointer) {
@@ -113,7 +113,7 @@ TEST_F(ResolverStorageClassValidationTest, StorageBufferPointer) {
     EXPECT_EQ(
         r()->error(),
         R"(56:78 error: Type 'ptr<private, f32, read_write>' cannot be used in storage class 'storage' as it is non-host-shareable
-56:78 note: while instantiating variable g)");
+56:78 note: while instantiating 'var' g)");
 }
 
 TEST_F(ResolverStorageClassValidationTest, StorageBufferIntScalar) {
@@ -166,7 +166,7 @@ TEST_F(ResolverStorageClassValidationTest, StorageBufferBoolAlias) {
     EXPECT_EQ(
         r()->error(),
         R"(56:78 error: Type 'bool' cannot be used in storage class 'storage' as it is non-host-shareable
-56:78 note: while instantiating variable g)");
+56:78 note: while instantiating 'var' g)");
 }
 
 TEST_F(ResolverStorageClassValidationTest, NotStorage_AccessMode) {
@@ -225,7 +225,7 @@ TEST_F(ResolverStorageClassValidationTest, UniformBuffer_Struct_Runtime) {
     EXPECT_EQ(r()->error(),
               R"(56:78 error: runtime-sized arrays can only be used in the <storage> storage class
 note: while analysing structure member S.m
-56:78 note: while instantiating variable svar)");
+56:78 note: while instantiating 'var' svar)");
 }
 
 TEST_F(ResolverStorageClassValidationTest, UniformBufferBool) {
@@ -241,7 +241,7 @@ TEST_F(ResolverStorageClassValidationTest, UniformBufferBool) {
     EXPECT_EQ(
         r()->error(),
         R"(56:78 error: Type 'bool' cannot be used in storage class 'uniform' as it is non-host-shareable
-56:78 note: while instantiating variable g)");
+56:78 note: while instantiating 'var' g)");
 }
 
 TEST_F(ResolverStorageClassValidationTest, UniformBufferPointer) {
@@ -258,7 +258,7 @@ TEST_F(ResolverStorageClassValidationTest, UniformBufferPointer) {
     EXPECT_EQ(
         r()->error(),
         R"(56:78 error: Type 'ptr<private, f32, read_write>' cannot be used in storage class 'uniform' as it is non-host-shareable
-56:78 note: while instantiating variable g)");
+56:78 note: while instantiating 'var' g)");
 }
 
 TEST_F(ResolverStorageClassValidationTest, UniformBufferIntScalar) {
@@ -314,7 +314,7 @@ TEST_F(ResolverStorageClassValidationTest, UniformBufferBoolAlias) {
     EXPECT_EQ(
         r()->error(),
         R"(56:78 error: Type 'bool' cannot be used in storage class 'uniform' as it is non-host-shareable
-56:78 note: while instantiating variable g)");
+56:78 note: while instantiating 'var' g)");
 }
 
 TEST_F(ResolverStorageClassValidationTest, UniformBufferNoError_Basic) {

src/tint/resolver/type_validation_test.cc

@@ -377,7 +377,7 @@ TEST_F(ResolverTypeValidationTest, RuntimeArrayInFunction_Fail) {
     EXPECT_FALSE(r()->Resolve());
     EXPECT_EQ(r()->error(),
               R"(12:34 error: runtime-sized arrays can only be used in the <storage> storage class
-12:34 note: while instantiating variable a)");
+12:34 note: while instantiating 'var' a)");
 }
 
 TEST_F(ResolverTypeValidationTest, Struct_Member_VectorNoType) {
@@ -528,7 +528,7 @@ TEST_F(ResolverTypeValidationTest, RuntimeArrayAsGlobalVariable) {
 
     EXPECT_EQ(r()->error(),
               R"(56:78 error: runtime-sized arrays can only be used in the <storage> storage class
-56:78 note: while instantiating variable g)");
+56:78 note: while instantiating 'var' g)");
 }
 
 TEST_F(ResolverTypeValidationTest, RuntimeArrayAsLocalVariable) {
@@ -539,7 +539,7 @@ TEST_F(ResolverTypeValidationTest, RuntimeArrayAsLocalVariable) {
 
     EXPECT_EQ(r()->error(),
               R"(56:78 error: runtime-sized arrays can only be used in the <storage> storage class
-56:78 note: while instantiating variable g)");
+56:78 note: while instantiating 'var' g)");
 }
 
 TEST_F(ResolverTypeValidationTest, RuntimeArrayAsParameter_Fail) {

src/tint/resolver/validator.cc

@@ -558,7 +558,7 @@ bool Validator::GlobalVariable(
                     return false;
                 }
             }
-            return true;
+            return Variable(global);
         },
         [&](const ast::Let*) {
             if (!decl->attributes.empty()) {
@@ -566,9 +566,14 @@ bool Validator::GlobalVariable(
                          decl->attributes[0]->source);
                 return false;
             }
-            return true;
+            return Variable(global);
         },
-        [&](const ast::Var*) {
+        [&](const ast::Var* var) {
+            if (global->StorageClass() == ast::StorageClass::kNone) {
+                AddError("module-scope 'var' declaration must have a storage class", decl->source);
+                return false;
+            }
+
             for (auto* attr : decl->attributes) {
                 bool is_shader_io_attribute =
                     attr->IsAnyOf<ast::BuiltinAttribute, ast::InterpolateAttribute,
@@ -582,7 +587,22 @@ bool Validator::GlobalVariable(
                     return false;
                 }
             }
-            return true;
+
+            // https://gpuweb.github.io/gpuweb/wgsl/#variable-declaration
+            // The access mode always has a default, and except for variables in the
+            // storage storage class, must not be written.
+            if (global->StorageClass() != ast::StorageClass::kStorage &&
+                var->declared_access != ast::Access::kUndefined) {
+                AddError("only variables in <storage> storage class may declare an access mode",
+                         var->source);
+                return false;
+            }
+
+            if (!AtomicVariable(global, atomic_composite_info)) {
+                return false;
+            }
+
+            return Var(global);
         });
 
     if (!ok) {
@@ -618,23 +638,7 @@ bool Validator::GlobalVariable(
             }
     }
 
-    if (auto* var = decl->As<ast::Var>()) {
+    return true;
-        // https://gpuweb.github.io/gpuweb/wgsl/#variable-declaration
-        // The access mode always has a default, and except for variables in the
-        // storage storage class, must not be written.
-        if (global->StorageClass() != ast::StorageClass::kStorage &&
-            var->declared_access != ast::Access::kUndefined) {
-            AddError("only variables in <storage> storage class may declare an access mode",
-                     var->source);
-            return false;
-        }
-
-        if (!AtomicVariable(global, atomic_composite_info)) {
-            return false;
-        }
-    }
-
-    return Variable(global);
 }
 
 // https://gpuweb.github.io/gpuweb/wgsl/#atomic-types
@@ -682,58 +686,67 @@ bool Validator::Variable(const sem::Variable* v) const {
     auto* decl = v->Declaration();
     auto* storage_ty = v->Type()->UnwrapRef();
 
-    auto* as_let = decl->As<ast::Let>();
+    auto* kind = decl->Is<ast::Override>() ? "'override'" : "'let'";
-    auto* as_var = decl->As<ast::Var>();
 
     if (v->Is<sem::GlobalVariable>()) {
         auto name = symbols_.NameFor(decl->symbol);
         if (sem::ParseBuiltinType(name) != sem::BuiltinType::kNone) {
-            auto* kind = as_let ? "let" : "var";
+            AddError("'" + name + "' is a builtin and cannot be redeclared as a " + kind,
-            AddError(
+                     decl->source);
-                "'" + name + "' is a builtin and cannot be redeclared as a module-scope " + kind,
-                decl->source);
             return false;
         }
     }
 
-    if (as_var && !IsStorable(storage_ty)) {
+    if (!(storage_ty->IsConstructible() || storage_ty->Is<sem::Pointer>())) {
-        AddError(sem_.TypeNameOf(storage_ty) + " cannot be used as the type of a var",
+        AddError(sem_.TypeNameOf(storage_ty) + " cannot be used as the type of a " + kind,
                  decl->source);
         return false;
     }
+    return true;
+}
 
-    if (as_let && !(storage_ty->IsConstructible() || storage_ty->Is<sem::Pointer>())) {
+bool Validator::Var(const sem::Variable* v) const {
-        AddError(sem_.TypeNameOf(storage_ty) + " cannot be used as the type of a let",
+    auto* var = v->Declaration()->As<ast::Var>();
-                 decl->source);
+    auto* storage_ty = v->Type()->UnwrapRef();
+
+    if (v->Is<sem::GlobalVariable>()) {
+        auto name = symbols_.NameFor(var->symbol);
+        if (sem::ParseBuiltinType(name) != sem::BuiltinType::kNone) {
+            AddError("'" + name + "' is a builtin and cannot be redeclared as a module-scope 'var'",
+                     var->source);
+            return false;
+        }
+    }
+
+    if (!IsStorable(storage_ty)) {
+        AddError(sem_.TypeNameOf(storage_ty) + " cannot be used as the type of a var", var->source);
         return false;
     }
 
-    if (v->Is<sem::LocalVariable>() && as_var &&
+    if (v->Is<sem::LocalVariable>() &&
-        IsValidationEnabled(decl->attributes, ast::DisabledValidation::kIgnoreStorageClass)) {
+        IsValidationEnabled(var->attributes, ast::DisabledValidation::kIgnoreStorageClass)) {
         if (!v->Type()->UnwrapRef()->IsConstructible()) {
-            AddError("function variable must have a constructible type",
+            AddError("function-scope 'var' must have a constructible type",
-                     decl->type ? decl->type->source : decl->source);
+                     var->type ? var->type->source : var->source);
             return false;
         }
     }
 
-    if (as_var && storage_ty->is_handle() &&
+    if (storage_ty->is_handle() && var->declared_storage_class != ast::StorageClass::kNone) {
-        as_var->declared_storage_class != ast::StorageClass::kNone) {
         // 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 attribute. The
         // storage class will always be handle.
         AddError(
             "variables of type '" + sem_.TypeNameOf(storage_ty) + "' must not have a storage class",
-            decl->source);
+            var->source);
         return false;
     }
 
-    if (IsValidationEnabled(decl->attributes, ast::DisabledValidation::kIgnoreStorageClass) &&
+    if (IsValidationEnabled(var->attributes, ast::DisabledValidation::kIgnoreStorageClass) &&
-        as_var &&
+        (var->declared_storage_class == ast::StorageClass::kInput ||
-        (as_var->declared_storage_class == ast::StorageClass::kInput ||
+         var->declared_storage_class == ast::StorageClass::kOutput)) {
-         as_var->declared_storage_class == ast::StorageClass::kOutput)) {
+        AddError("invalid use of input/output storage class", var->source);
-        AddError("invalid use of input/output storage class", as_var->source);
         return false;
     }
     return true;

src/tint/resolver/validator.h

@@ -352,9 +352,14 @@ class Validator {
     bool SwitchStatement(const ast::SwitchStatement* s);
 
     /// Validates a variable
-    /// @param var the variable to validate
+    /// @param v the variable to validate
     /// @returns true on success, false otherwise.
-    bool Variable(const sem::Variable* var) const;
+    bool Variable(const sem::Variable* v) const;
+
+    /// Validates a 'var' variable declaration
+    /// @param v the variable to validate
+    /// @returns true on success, false otherwise.
+    bool Var(const sem::Variable* v) const;
 
     /// Validates a variable constructor or cast
     /// @param v the variable to validate

src/tint/resolver/var_let_validation_test.cc

@@ -29,8 +29,7 @@ TEST_F(ResolverVarLetValidationTest, VarNoInitializerNoType) {
     WrapInFunction(Var(Source{{12, 34}}, "a", nullptr));
 
     EXPECT_FALSE(r()->Resolve());
-    EXPECT_EQ(r()->error(),
+    EXPECT_EQ(r()->error(), "12:34 error: 'var' declaration requires a type or initializer");
-              "12:34 error: function-scope 'var' declaration requires a type or initializer");
 }
 
 TEST_F(ResolverVarLetValidationTest, GlobalVarNoInitializerNoType) {
@@ -38,8 +37,15 @@ TEST_F(ResolverVarLetValidationTest, GlobalVarNoInitializerNoType) {
     Global(Source{{12, 34}}, "a", nullptr);
 
     EXPECT_FALSE(r()->Resolve());
-    EXPECT_EQ(r()->error(),
+    EXPECT_EQ(r()->error(), "12:34 error: 'var' declaration requires a type or initializer");
-              "12:34 error: module-scope 'var' declaration requires a type or initializer");
+}
+
+TEST_F(ResolverVarLetValidationTest, OverrideNoInitializerNoType) {
+    // override a;
+    Override(Source{{12, 34}}, "a", nullptr, nullptr);
+
+    EXPECT_FALSE(r()->Resolve());
+    EXPECT_EQ(r()->error(), "12:34 error: 'override' declaration requires a type or initializer");
 }
 
 TEST_F(ResolverVarLetValidationTest, VarTypeNotStorable) {
@@ -65,7 +71,7 @@ TEST_F(ResolverVarLetValidationTest, LetTypeNotConstructible) {
     WrapInFunction(t2);
 
     EXPECT_FALSE(r()->Resolve());
-    EXPECT_EQ(r()->error(), "56:78 error: texture_2d<f32> cannot be used as the type of a let");
+    EXPECT_EQ(r()->error(), "56:78 error: texture_2d<f32> cannot be used as the type of a 'let'");
 }
 
 TEST_F(ResolverVarLetValidationTest, LetConstructorWrongType) {
@@ -217,7 +223,7 @@ TEST_F(ResolverVarLetValidationTest, NonConstructibleType_Atomic) {
     WrapInFunction(v);
 
     EXPECT_FALSE(r()->Resolve());
-    EXPECT_EQ(r()->error(), "12:34 error: function variable must have a constructible type");
+    EXPECT_EQ(r()->error(), "12:34 error: function-scope 'var' must have a constructible type");
 }
 
 TEST_F(ResolverVarLetValidationTest, NonConstructibleType_RuntimeArray) {
@@ -229,7 +235,7 @@ TEST_F(ResolverVarLetValidationTest, NonConstructibleType_RuntimeArray) {
     EXPECT_EQ(r()->error(),
               R"(12:34 error: runtime-sized arrays can only be used in the <storage> storage class
 56:78 note: while analysing structure member S.m
-12:34 note: while instantiating variable v)");
+12:34 note: while instantiating 'var' v)");
 }
 
 TEST_F(ResolverVarLetValidationTest, NonConstructibleType_Struct_WithAtomic) {
@@ -238,7 +244,7 @@ TEST_F(ResolverVarLetValidationTest, NonConstructibleType_Struct_WithAtomic) {
     WrapInFunction(v);
 
     EXPECT_FALSE(r()->Resolve());
-    EXPECT_EQ(r()->error(), "error: function variable must have a constructible type");
+    EXPECT_EQ(r()->error(), "error: function-scope 'var' must have a constructible type");
 }
 
 TEST_F(ResolverVarLetValidationTest, NonConstructibleType_InferredType) {
@@ -251,7 +257,7 @@ TEST_F(ResolverVarLetValidationTest, NonConstructibleType_InferredType) {
     WrapInFunction(v);
 
     EXPECT_FALSE(r()->Resolve());
-    EXPECT_EQ(r()->error(), "12:34 error: function variable must have a constructible type");
+    EXPECT_EQ(r()->error(), "12:34 error: function-scope 'var' must have a constructible type");
 }
 
 TEST_F(ResolverVarLetValidationTest, InvalidStorageClassForInitializer) {