tint/resolver: Clean up 'let' / 'override' resolving
Split the logic out of Resolver::Variable().
Primes the resolver for the introduction of 'const'
Bug: tint:1580
Change-Id: Id67280ed5c8c73a69c62728fb5a81a08f13628a8
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/93785
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc
index 8d3cfdd..dcd49fb 100644
--- a/src/tint/resolver/resolver.cc
+++ b/src/tint/resolver/resolver.cc
@@ -312,6 +312,20 @@
}
sem::Variable* Resolver::Variable(const ast::Variable* v, bool is_global) {
+ return Switch(
+ v, //
+ [&](const ast::Var* var) { return Var(var, is_global); },
+ [&](const ast::Let* let) { return Let(let, is_global); },
+ [&](const ast::Override* override) { return Override(override); },
+ [&](Default) {
+ TINT_ICE(Resolver, diagnostics_)
+ << "Resolver::GlobalVariable() called with a unknown variable type: "
+ << v->TypeInfo().name;
+ return nullptr;
+ });
+}
+
+sem::Variable* Resolver::Let(const ast::Let* v, bool is_global) {
const sem::Type* ty = nullptr;
// If the variable has a declared type, resolve it.
@@ -322,8 +336,58 @@
}
}
- auto* as_let = v->As<ast::Let>();
- auto* as_override = v->As<ast::Override>();
+ if (!v->constructor) {
+ AddError("'let' declaration must have an initializer", v->source);
+ return nullptr;
+ }
+
+ auto* rhs = Materialize(Expression(v->constructor), ty);
+ if (!rhs) {
+ return nullptr;
+ }
+
+ // If the variable has no declared type, infer it from the RHS
+ if (!ty) {
+ ty = rhs->Type()->UnwrapRef(); // Implicit load of RHS
+ }
+
+ 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 'let' " + builder_->Symbols().NameFor(v->symbol), v->source);
+ return nullptr;
+ }
+
+ sem::Variable* sem = nullptr;
+ if (is_global) {
+ sem = builder_->create<sem::GlobalVariable>(
+ v, ty, ast::StorageClass::kNone, ast::Access::kUndefined,
+ rhs ? rhs->ConstantValue() : sem::Constant{}, sem::BindingPoint{});
+ } else {
+ sem = builder_->create<sem::LocalVariable>(v, ty, ast::StorageClass::kNone,
+ ast::Access::kUndefined, current_statement_,
+ rhs ? rhs->ConstantValue() : sem::Constant{});
+ }
+
+ sem->SetConstructor(rhs);
+ builder_->Sem().Add(v, sem);
+ return sem;
+}
+
+sem::Variable* Resolver::Override(const ast::Override* v) {
+ const sem::Type* ty = nullptr;
+
+ // If the variable has a declared type, resolve it.
+ if (v->type) {
+ ty = Type(v->type);
+ if (!ty) {
+ return nullptr;
+ }
+ }
const sem::Expression* rhs = nullptr;
@@ -338,9 +402,6 @@
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 (!ty) {
AddError("'override' declaration requires a type or initializer", v->source);
return nullptr;
@@ -353,28 +414,17 @@
if (!ApplyStorageClassUsageToType(ast::StorageClass::kNone, const_cast<sem::Type*>(ty),
v->source)) {
- AddNote("while instantiating variable " + builder_->Symbols().NameFor(v->symbol),
+ AddNote("while instantiating 'override' " + 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{});
+ auto* sem = builder_->create<sem::GlobalVariable>(v, ty, ast::StorageClass::kNone,
+ ast::Access::kUndefined, 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{});
+ if (auto* id = ast::GetAttribute<ast::IdAttribute>(v->attributes)) {
+ sem->SetConstantId(static_cast<uint16_t>(id->value));
}
sem->SetConstructor(rhs);
@@ -567,10 +617,7 @@
}
sem::GlobalVariable* Resolver::GlobalVariable(const ast::Variable* v) {
- 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));
+ auto* sem = As<sem::GlobalVariable>(Variable(v, /* is_global */ true));
if (!sem) {
return nullptr;
}
@@ -2477,10 +2524,7 @@
return StatementScope(stmt, sem, [&] {
Mark(stmt->variable);
- auto* variable = Switch(
- stmt->variable, //
- [&](const ast::Var* var) { return Var(var, /* is_global */ false); },
- [&](Default) { return Variable(stmt->variable, /* is_global */ false); });
+ auto* variable = Variable(stmt->variable, /* is_global */ false);
if (!variable) {
return false;
}
@@ -2501,10 +2545,7 @@
sem->Behaviors() = ctor->Behaviors();
}
- return Switch(
- stmt->variable, //
- [&](const ast::Var*) { return validator_.Var(variable); },
- [&](Default) { return validator_.Variable(variable); });
+ return validator_.Variable(variable);
});
}
diff --git a/src/tint/resolver/resolver.h b/src/tint/resolver/resolver.h
index faaa2d4..5b73e9d 100644
--- a/src/tint/resolver/resolver.h
+++ b/src/tint/resolver/resolver.h
@@ -298,6 +298,21 @@
/// @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::Let` `v`. 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* Let(const ast::Let* var, bool is_global);
+
+ /// @returns the semantic info for the module-scope `ast::Override` `v`. If an error is raised,
+ /// nullptr is returned.
+ /// @note this method does not resolve the attributes as these are context-dependent (global,
+ /// local)
+ /// @param override the variable
+ sem::Variable* Override(const ast::Override* override);
+
/// @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,
diff --git a/src/tint/resolver/validator.cc b/src/tint/resolver/validator.cc
index 53ce696..b12143f 100644
--- a/src/tint/resolver/validator.cc
+++ b/src/tint/resolver/validator.cc
@@ -558,7 +558,7 @@
return false;
}
}
- return Variable(global);
+ return Override(global);
},
[&](const ast::Let*) {
if (!decl->attributes.empty()) {
@@ -566,7 +566,7 @@
decl->attributes[0]->source);
return false;
}
- return Variable(global);
+ return Let(global);
},
[&](const ast::Var* var) {
if (global->StorageClass() == ast::StorageClass::kNone) {
@@ -684,21 +684,53 @@
bool Validator::Variable(const sem::Variable* v) const {
auto* decl = v->Declaration();
- auto* storage_ty = v->Type()->UnwrapRef();
+ return Switch(
+ decl, //
+ [&](const ast::Var*) { return Var(v); }, //
+ [&](const ast::Let*) { return Let(v); }, //
+ [&](const ast::Override*) { return Override(v); }, //
+ [&](Default) {
+ TINT_ICE(Resolver, diagnostics_)
+ << "Validator::Variable() called with a unknown variable type: "
+ << decl->TypeInfo().name;
+ return false;
+ });
+}
- auto* kind = decl->Is<ast::Override>() ? "'override'" : "'let'";
+bool Validator::Let(const sem::Variable* v) const {
+ auto* decl = v->Declaration();
+ auto* storage_ty = v->Type()->UnwrapRef();
if (v->Is<sem::GlobalVariable>()) {
auto name = symbols_.NameFor(decl->symbol);
if (sem::ParseBuiltinType(name) != sem::BuiltinType::kNone) {
- AddError("'" + name + "' is a builtin and cannot be redeclared as a " + kind,
+ AddError("'" + name + "' is a builtin and cannot be redeclared as a 'let'",
decl->source);
return false;
}
}
if (!(storage_ty->IsConstructible() || storage_ty->Is<sem::Pointer>())) {
- AddError(sem_.TypeNameOf(storage_ty) + " cannot be used as the type of a " + kind,
+ AddError(sem_.TypeNameOf(storage_ty) + " cannot be used as the type of a 'let'",
+ decl->source);
+ return false;
+ }
+ return true;
+}
+
+bool Validator::Override(const sem::Variable* v) const {
+ auto* decl = v->Declaration();
+ auto* storage_ty = v->Type()->UnwrapRef();
+
+ auto name = symbols_.NameFor(decl->symbol);
+ if (sem::ParseBuiltinType(name) != sem::BuiltinType::kNone) {
+ AddError("'" + name + "' is a builtin and cannot be redeclared as a 'override'",
+ decl->source);
+ return false;
+ }
+
+ if (!storage_ty->is_scalar()) {
+ AddError(sem_.TypeNameOf(storage_ty) + " cannot be used as the type of a 'override'",
decl->source);
return false;
}
diff --git a/src/tint/resolver/validator.h b/src/tint/resolver/validator.h
index f417557..8437752 100644
--- a/src/tint/resolver/validator.h
+++ b/src/tint/resolver/validator.h
@@ -356,6 +356,16 @@
/// @returns true on success, false otherwise.
bool Variable(const sem::Variable* v) const;
+ /// Validates a 'let' variable declaration
+ /// @param v the variable to validate
+ /// @returns true on success, false otherwise.
+ bool Let(const sem::Variable* v) const;
+
+ /// Validates a 'override' variable declaration
+ /// @param v the variable to validate
+ /// @returns true on success, false otherwise.
+ bool Override(const sem::Variable* v) const;
+
/// Validates a 'var' variable declaration
/// @param v the variable to validate
/// @returns true on success, false otherwise.
diff --git a/src/tint/resolver/var_let_validation_test.cc b/src/tint/resolver/var_let_validation_test.cc
index 6b9df5d..5d05c9a 100644
--- a/src/tint/resolver/var_let_validation_test.cc
+++ b/src/tint/resolver/var_let_validation_test.cc
@@ -74,6 +74,22 @@
EXPECT_EQ(r()->error(), "56:78 error: texture_2d<f32> cannot be used as the type of a 'let'");
}
+TEST_F(ResolverVarLetValidationTest, OverrideExplicitTypeNotScalar) {
+ // override o : vec3<f32>;
+ Override(Source{{56, 78}}, "o", ty.vec3<f32>(), nullptr);
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(), "56:78 error: vec3<f32> cannot be used as the type of a 'override'");
+}
+
+TEST_F(ResolverVarLetValidationTest, OverrideInferedTypeNotScalar) {
+ // override o = vec3(1.0f);
+ Override(Source{{56, 78}}, "o", nullptr, vec3<f32>(1.0_f));
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(), "56:78 error: vec3<f32> cannot be used as the type of a 'override'");
+}
+
TEST_F(ResolverVarLetValidationTest, LetConstructorWrongType) {
// var v : i32 = 2u
WrapInFunction(Let(Source{{3, 3}}, "v", ty.i32(), Expr(2_u)));
