From e4e4854b77c3bce27996d86cbcfbed0b158397df Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Mon, 20 Jun 2022 19:17:52 +0000 Subject: [PATCH] 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 Commit-Queue: Ben Clayton Reviewed-by: Dan Sinclair --- src/tint/resolver/resolver.cc | 109 +++++++++++++------ src/tint/resolver/resolver.h | 15 +++ src/tint/resolver/validator.cc | 44 +++++++- src/tint/resolver/validator.h | 10 ++ src/tint/resolver/var_let_validation_test.cc | 16 +++ 5 files changed, 154 insertions(+), 40 deletions(-) diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc index 8d3cfdd429..dcd49fbf77 100644 --- a/src/tint/resolver/resolver.cc +++ b/src/tint/resolver/resolver.cc @@ -312,6 +312,20 @@ sem::Type* Resolver::Type(const ast::Type* ty) { } 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 @@ sem::Variable* Resolver::Variable(const ast::Variable* v, bool is_global) { } } - auto* as_let = v->As(); - auto* as_override = v->As(); + 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(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( + v, ty, ast::StorageClass::kNone, ast::Access::kUndefined, + rhs ? rhs->ConstantValue() : sem::Constant{}, sem::BindingPoint{}); + } else { + sem = builder_->create(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 @@ sem::Variable* Resolver::Variable(const ast::Variable* v, bool is_global) { 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 @@ sem::Variable* Resolver::Variable(const ast::Variable* v, bool is_global) { if (!ApplyStorageClassUsageToType(ast::StorageClass::kNone, const_cast(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( - v, ty, ast::StorageClass::kNone, ast::Access::kUndefined, - has_const_val ? rhs->ConstantValue() : sem::Constant{}, sem::BindingPoint{}); + auto* sem = builder_->create(v, ty, ast::StorageClass::kNone, + ast::Access::kUndefined, sem::Constant{}, + sem::BindingPoint{}); - if (as_override) { - if (auto* id = ast::GetAttribute(v->attributes)) { - global->SetConstantId(static_cast(id->value)); - } - } - sem = global; - } else { - sem = builder_->create( - v, ty, ast::StorageClass::kNone, ast::Access::kUndefined, current_statement_, - (rhs && as_let) ? rhs->ConstantValue() : sem::Constant{}); + if (auto* id = ast::GetAttribute(v->attributes)) { + sem->SetConstantId(static_cast(id->value)); } sem->SetConstructor(rhs); @@ -567,10 +617,7 @@ void Resolver::SetShadows() { } sem::GlobalVariable* Resolver::GlobalVariable(const ast::Variable* v) { - auto* as_var = v->As(); - - auto* sem = As(as_var ? Var(as_var, /* is_global */ true) - : Variable(v, /* is_global */ true)); + auto* sem = As(Variable(v, /* is_global */ true)); if (!sem) { return nullptr; } @@ -2477,10 +2524,7 @@ sem::Statement* Resolver::VariableDeclStatement(const ast::VariableDeclStatement 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::Statement* Resolver::VariableDeclStatement(const ast::VariableDeclStatement 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 faaa2d47c9..5b73e9d819 100644 --- a/src/tint/resolver/resolver.h +++ b/src/tint/resolver/resolver.h @@ -298,6 +298,21 @@ 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::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 53ce696bf5..b12143f4e5 100644 --- a/src/tint/resolver/validator.cc +++ b/src/tint/resolver/validator.cc @@ -558,7 +558,7 @@ bool Validator::GlobalVariable( return false; } } - return Variable(global); + return Override(global); }, [&](const ast::Let*) { if (!decl->attributes.empty()) { @@ -566,7 +566,7 @@ bool Validator::GlobalVariable( 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::AtomicVariable( 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() ? "'override'" : "'let'"; +bool Validator::Let(const sem::Variable* v) const { + auto* decl = v->Declaration(); + auto* storage_ty = v->Type()->UnwrapRef(); if (v->Is()) { 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())) { - 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 f417557dba..8437752ca5 100644 --- a/src/tint/resolver/validator.h +++ b/src/tint/resolver/validator.h @@ -356,6 +356,16 @@ class Validator { /// @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 6b9df5d3eb..5d05c9a609 100644 --- a/src/tint/resolver/var_let_validation_test.cc +++ b/src/tint/resolver/var_let_validation_test.cc @@ -74,6 +74,22 @@ TEST_F(ResolverVarLetValidationTest, LetTypeNotConstructible) { EXPECT_EQ(r()->error(), "56:78 error: texture_2d cannot be used as the type of a 'let'"); } +TEST_F(ResolverVarLetValidationTest, OverrideExplicitTypeNotScalar) { + // override o : vec3; + Override(Source{{56, 78}}, "o", ty.vec3(), nullptr); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), "56:78 error: vec3 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(1.0_f)); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), "56:78 error: vec3 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)));