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>
diff --git a/src/tint/resolver/atomics_validation_test.cc b/src/tint/resolver/atomics_validation_test.cc index 78c1fb9..adc830e 100644 --- a/src/tint/resolver/atomics_validation_test.cc +++ b/src/tint/resolver/atomics_validation_test.cc
@@ -309,7 +309,7 @@ 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
diff --git a/src/tint/resolver/builtin_validation_test.cc b/src/tint/resolver/builtin_validation_test.cc index d82c2f3..24ead62 100644 --- a/src/tint/resolver/builtin_validation_test.cc +++ b/src/tint/resolver/builtin_validation_test.cc
@@ -90,7 +90,7 @@ 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 @@ 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) {
diff --git a/src/tint/resolver/host_shareable_validation_test.cc b/src/tint/resolver/host_shareable_validation_test.cc index 01fbfb0..85ccd85 100644 --- a/src/tint/resolver/host_shareable_validation_test.cc +++ b/src/tint/resolver/host_shareable_validation_test.cc
@@ -38,7 +38,7 @@ 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 @@ 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 @@ 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 @@ 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) {
diff --git a/src/tint/resolver/inferred_type_test.cc b/src/tint/resolver/inferred_type_test.cc index 4d01f7e..d5912c2 100644 --- a/src/tint/resolver/inferred_type_test.cc +++ b/src/tint/resolver/inferred_type_test.cc
@@ -89,16 +89,18 @@ 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_EQ(r()->error(), "12:34 error: module-scope 'var' declaration must specify a type"); + EXPECT_TRUE(r()->Resolve()) << r()->error(); + EXPECT_EQ(TypeOf(var)->UnwrapRef(), expected_type); } TEST_P(ResolverInferredTypeParamTest, LocalLet_Pass) {
diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc index be1765c..8d3cfdd 100644 --- a/src/tint/resolver/resolver.cc +++ b/src/tint/resolver/resolver.cc
@@ -312,17 +312,16 @@ } 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) { - storage_ty = Type(ty); - if (!storage_ty) { + if (v->type) { + ty = Type(v->type); + 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 @@ // 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 (as_var && is_global) { - AddError("module-scope 'var' declaration must specify a type", v->source); - return nullptr; - } - - storage_ty = rhs->Type()->UnwrapRef(); // Implicit load of RHS + if (!ty) { + 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) { - AddError((is_global) ? "module-scope 'var' declaration requires a type or initializer" - : "function-scope 'var' declaration requires a type or initializer", - v->source); + } else if (!ty) { + AddError("'override' 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 '" + - builder_->Symbols().NameFor(v->symbol) + "'\n" - << "Source: " << v->source; + AddError("'var' declaration requires a type or initializer", var->source); return nullptr; } - auto storage_class = as_var ? as_var->declared_storage_class : ast::StorageClass::kNone; - if (storage_class == ast::StorageClass::kNone && as_var) { + auto storage_class = var->declared_storage_class; + 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 @@ } } - if (!is_global && as_var && storage_class != ast::StorageClass::kFunction && - validator_.IsValidationEnabled(v->attributes, + if (!is_global && storage_class != ast::StorageClass::kFunction && + 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 (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())) { + if (rhs && !validator_.VariableConstructorOrCast(var, storage_class, storage_ty, rhs->Type())) { return nullptr; } - if (!ApplyStorageClassUsageToType(storage_class, const_cast<sem::Type*>(var_ty), v->source)) { - AddNote("while instantiating variable " + builder_->Symbols().NameFor(v->symbol), - v->source); + auto* var_ty = builder_->create<sem::Reference>(storage_ty, storage_class, access); + + 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 = as_var->BindingPoint()) { - binding_point = {bp.group->value, bp.binding->value}; - } + if (auto bp = var->BindingPoint()) { + 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; - auto* global = builder_->create<sem::GlobalVariable>( - v, var_ty, storage_class, access, - 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; + } else { + sem = builder_->create<sem::LocalVariable>(var, var_ty, storage_class, access, + current_statement_, sem::Constant{}); } - auto* local = builder_->create<sem::LocalVariable>( - v, var_ty, storage_class, access, current_statement_, - (rhs && as_let) ? rhs->ConstantValue() : sem::Constant{}); - builder_->Sem().Add(v, local); - local->SetConstructor(rhs); - return local; + sem->SetConstructor(rhs); + builder_->Sem().Add(var, sem); + return sem; } sem::Parameter* Resolver::Parameter(const ast::Parameter* param, uint32_t index) { @@ -534,19 +567,14 @@ } 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 @@ return nullptr; } - return sem->As<sem::GlobalVariable>(); + return sem; } sem::Function* Resolver::Function(const ast::Function* decl) { @@ -2449,8 +2477,11 @@ return StatementScope(stmt, sem, [&] { Mark(stmt->variable); - auto* var = Variable(stmt->variable, /* is_global */ false); - if (!var) { + auto* variable = Switch( + 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 @@ 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); }); }); }
diff --git a/src/tint/resolver/resolver.h b/src/tint/resolver/resolver.h index e75c952..faaa2d4 100644 --- a/src/tint/resolver/resolver.h +++ b/src/tint/resolver/resolver.h
@@ -298,6 +298,14 @@ /// @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
diff --git a/src/tint/resolver/storage_class_validation_test.cc b/src/tint/resolver/storage_class_validation_test.cc index a5e7d12..e161127 100644 --- a/src/tint/resolver/storage_class_validation_test.cc +++ b/src/tint/resolver/storage_class_validation_test.cc
@@ -49,7 +49,7 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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) {
diff --git a/src/tint/resolver/type_validation_test.cc b/src/tint/resolver/type_validation_test.cc index ec2f8f5..e79026e 100644 --- a/src/tint/resolver/type_validation_test.cc +++ b/src/tint/resolver/type_validation_test.cc
@@ -377,7 +377,7 @@ 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 @@ 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 @@ 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) {
diff --git a/src/tint/resolver/validator.cc b/src/tint/resolver/validator.cc index e03aa13..53ce696 100644 --- a/src/tint/resolver/validator.cc +++ b/src/tint/resolver/validator.cc
@@ -558,7 +558,7 @@ return false; } } - return true; + return Variable(global); }, [&](const ast::Let*) { if (!decl->attributes.empty()) { @@ -566,9 +566,14 @@ 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 @@ 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 @@ } } - if (auto* var = decl->As<ast::Var>()) { - // 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); + return true; } // https://gpuweb.github.io/gpuweb/wgsl/#atomic-types @@ -682,58 +686,67 @@ auto* decl = v->Declaration(); auto* storage_ty = v->Type()->UnwrapRef(); - auto* as_let = decl->As<ast::Let>(); - auto* as_var = decl->As<ast::Var>(); + auto* kind = decl->Is<ast::Override>() ? "'override'" : "'let'"; 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 module-scope " + kind, - decl->source); + AddError("'" + name + "' is a builtin and cannot be redeclared as a " + kind, + decl->source); return false; } } - if (as_var && !IsStorable(storage_ty)) { - AddError(sem_.TypeNameOf(storage_ty) + " cannot be used as the type of a var", + if (!(storage_ty->IsConstructible() || storage_ty->Is<sem::Pointer>())) { + 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>())) { - AddError(sem_.TypeNameOf(storage_ty) + " cannot be used as the type of a let", - decl->source); +bool Validator::Var(const sem::Variable* v) const { + auto* var = v->Declaration()->As<ast::Var>(); + 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 && - IsValidationEnabled(decl->attributes, ast::DisabledValidation::kIgnoreStorageClass)) { + if (v->Is<sem::LocalVariable>() && + IsValidationEnabled(var->attributes, ast::DisabledValidation::kIgnoreStorageClass)) { if (!v->Type()->UnwrapRef()->IsConstructible()) { - AddError("function variable must have a constructible type", - decl->type ? decl->type->source : decl->source); + AddError("function-scope 'var' must have a constructible type", + var->type ? var->type->source : var->source); return false; } } - if (as_var && storage_ty->is_handle() && - as_var->declared_storage_class != ast::StorageClass::kNone) { + if (storage_ty->is_handle() && 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) && - as_var && - (as_var->declared_storage_class == ast::StorageClass::kInput || - as_var->declared_storage_class == ast::StorageClass::kOutput)) { - AddError("invalid use of input/output storage class", as_var->source); + if (IsValidationEnabled(var->attributes, ast::DisabledValidation::kIgnoreStorageClass) && + (var->declared_storage_class == ast::StorageClass::kInput || + var->declared_storage_class == ast::StorageClass::kOutput)) { + AddError("invalid use of input/output storage class", var->source); return false; } return true;
diff --git a/src/tint/resolver/validator.h b/src/tint/resolver/validator.h index 30dcaf9..f417557 100644 --- a/src/tint/resolver/validator.h +++ b/src/tint/resolver/validator.h
@@ -352,9 +352,14 @@ 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
diff --git a/src/tint/resolver/var_let_validation_test.cc b/src/tint/resolver/var_let_validation_test.cc index 67ecd53..6b9df5d 100644 --- a/src/tint/resolver/var_let_validation_test.cc +++ b/src/tint/resolver/var_let_validation_test.cc
@@ -29,8 +29,7 @@ WrapInFunction(Var(Source{{12, 34}}, "a", nullptr)); EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), - "12:34 error: function-scope 'var' declaration requires a type or initializer"); + EXPECT_EQ(r()->error(), "12:34 error: 'var' declaration requires a type or initializer"); } TEST_F(ResolverVarLetValidationTest, GlobalVarNoInitializerNoType) { @@ -38,8 +37,15 @@ Global(Source{{12, 34}}, "a", nullptr); EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), - "12:34 error: module-scope 'var' declaration requires a type or initializer"); + EXPECT_EQ(r()->error(), "12:34 error: '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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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) {