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) {