Resolver: clean up how NamedTypes are handled Also add validation for when a named type is declared more than once. Bug: tint:803 Change-Id: Ifa93b34bc5afd31eba9bfdc4514791ec07c767ec Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51622 Commit-Queue: Ben Clayton <bclayton@chromium.org> Kokoro: Kokoro <noreply+kokoro@google.com> Reviewed-by: Ben Clayton <bclayton@chromium.org>
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc index d05d058..fcfc908 100644 --- a/src/resolver/resolver.cc +++ b/src/resolver/resolver.cc
@@ -212,28 +212,12 @@ bool Resolver::ResolveInternal() { Mark(&builder_->AST()); - auto register_named_type = [this](Symbol name, sem::Type* type, - const Source& source) { - auto added = named_types_.emplace(name, type).second; - if (!added) { - diagnostics_.add_error("type with the name '" + - builder_->Symbols().NameFor(name) + - "' was already declared", - source); - return false; - } - return true; - }; - // Process everything else in the order they appear in the module. This is // necessary for validation of use-before-declaration. for (auto* decl : builder_->AST().GlobalDeclarations()) { if (auto* ty = decl->As<ast::NamedType>()) { - auto* sem_ty = Type(ty); - if (sem_ty == nullptr) { - return false; - } - if (!register_named_type(ty->name(), sem_ty, ty->source())) { + Mark(ty); + if (!NamedType(ty)) { return false; } } else if (auto* func = decl->As<ast::Function>()) { @@ -299,14 +283,6 @@ if (ty->Is<ast::F32>()) { return builder_->create<sem::F32>(); } - if (auto* t = ty->As<ast::Alias>()) { - auto added = name_to_ast_type_.emplace(t->name(), t->type()).second; - // TODO(crbug.com/tint/803): Remove this. - if (!added) { - return nullptr; - } - return Type(t->type()); - } if (auto* t = ty->As<ast::AccessControl>()) { TINT_SCOPED_ASSIGNMENT(curent_access_control_, t); if (auto* el = Type(t->type())) { @@ -339,9 +315,6 @@ } return nullptr; } - if (auto* t = ty->As<ast::Struct>()) { - return Structure(t); - } if (auto* t = ty->As<ast::Sampler>()) { return builder_->create<sem::Sampler>(t->kind()); } @@ -383,14 +356,14 @@ return builder_->create<sem::ExternalTexture>(); } if (auto* t = ty->As<ast::TypeName>()) { - auto it = named_types_.find(t->name()); - if (it == named_types_.end()) { + auto it = named_type_info_.find(t->name()); + if (it == named_type_info_.end()) { diagnostics_.add_error( "unknown type '" + builder_->Symbols().NameFor(t->name()) + "'", t->source()); return nullptr; } - return it->second; + return it->second.sem; } TINT_UNREACHABLE(diagnostics_) << "Unhandled ast::Type: " << ty->TypeInfo().name; @@ -488,19 +461,23 @@ // TODO(crbug.com/tint/802): Temporary while ast::AccessControl exits. auto find_first_access_control = - [this](ast::Type* ty) -> ast::AccessControl* { + [this](const ast::Type* ty) -> const ast::AccessControl* { if (ty == nullptr) { return nullptr; } - if (ast::AccessControl* ac = ty->As<ast::AccessControl>()) { + if (const ast::AccessControl* ac = ty->As<ast::AccessControl>()) { return ac; } while (auto* tn = ty->As<ast::TypeName>()) { - auto it = name_to_ast_type_.find(tn->name()); - if (it == name_to_ast_type_.end()) { + auto it = named_type_info_.find(tn->name()); + if (it == named_type_info_.end()) { break; } - ty = it->second; + auto* alias = it->second.ast->As<ast::Alias>(); + if (!alias) { + break; + } + ty = alias->type(); if (auto* ac = ty->As<ast::AccessControl>()) { return ac; } @@ -2425,6 +2402,47 @@ return true; } +sem::Type* Resolver::NamedType(const ast::NamedType* named_type) { + sem::Type* result = nullptr; + if (auto* alias = named_type->As<ast::Alias>()) { + result = Type(alias->type()); + } else if (auto* str = named_type->As<ast::Struct>()) { + result = Structure(str); + } else { + TINT_UNREACHABLE(diagnostics_) << "Unhandled NamedType"; + } + + if (!result) { + return nullptr; + } + + named_type_info_.emplace(named_type->name(), + NamedTypeInfo{named_type, result}); + + if (!ValidateNamedType(named_type)) { + return nullptr; + } + + builder_->Sem().Add(named_type, result); + return result; +} + +bool Resolver::ValidateNamedType(const ast::NamedType* named_type) const { + auto iter = named_type_info_.find(named_type->name()); + if (iter == named_type_info_.end()) { + TINT_ICE(diagnostics_) << "ValidateNamedType called() before NamedType()"; + } + if (iter->second.ast != named_type) { + diagnostics_.add_error("type with the name '" + + builder_->Symbols().NameFor(named_type->name()) + + "' was already declared", + named_type->source()); + diagnostics_.add_note("first declared here", iter->second.ast->source()); + return false; + } + return true; +} + sem::Type* Resolver::TypeOf(const ast::Expression* expr) { auto it = expr_info_.find(expr); if (it != expr_info_.end()) {
diff --git a/src/resolver/resolver.h b/src/resolver/resolver.h index 8a622d2..ac2bbf0 100644 --- a/src/resolver/resolver.h +++ b/src/resolver/resolver.h
@@ -169,6 +169,12 @@ size_t first_continue = kNoContinue; }; + // Structure holding information for a NamedType + struct NamedTypeInfo { + ast::NamedType const* const ast; + sem::Type* const sem; + }; + /// Describes the context in which a variable is declared enum class VariableKind { kParameter, kLocal, kGlobal }; @@ -247,6 +253,7 @@ const std::string& rhs_type_name); bool ValidateVectorConstructor(const ast::TypeConstructorExpression* ctor, const sem::Vector* vec_type); + bool ValidateNamedType(const ast::NamedType* named_type) const; /// @returns the sem::Type for the ast::Type `ty`, building it if it /// hasn't been constructed already. If an error is raised, nullptr is @@ -254,6 +261,10 @@ /// @param ty the ast::Type sem::Type* Type(const ast::Type* ty); + /// @param named_type the named type to resolve + /// @returns the resolved semantic type + sem::Type* NamedType(const ast::NamedType* named_type); + /// Builds and returns the semantic information for the array `arr`. /// This method does not mark the ast::Array node, nor attach the generated /// semantic information to the AST node. @@ -358,10 +369,10 @@ std::unordered_map<const ast::Variable*, VariableInfo*> variable_to_info_; std::unordered_map<ast::CallExpression*, FunctionCallInfo> function_calls_; std::unordered_map<const ast::Expression*, ExpressionInfo> expr_info_; - std::unordered_map<Symbol, sem::Type*> named_types_; + std::unordered_map<Symbol, NamedTypeInfo> named_type_info_; + std::unordered_set<const ast::Node*> marked_; std::unordered_map<uint32_t, const VariableInfo*> constant_ids_; - std::unordered_map<Symbol, ast::Type*> name_to_ast_type_; FunctionInfo* current_function_ = nullptr; sem::Statement* current_statement_ = nullptr;
diff --git a/src/resolver/resolver_test.cc b/src/resolver/resolver_test.cc index 8e5631b..53fb0b7 100644 --- a/src/resolver/resolver_test.cc +++ b/src/resolver/resolver_test.cc
@@ -277,6 +277,19 @@ EXPECT_TRUE(TypeOf(init)->Is<sem::I32>()); } +TEST_F(ResolverTest, Stmt_VariableDecl_AliasRedeclared) { + auto* my_int1 = ty.alias(Source{{12, 34}}, "MyInt", ty.i32()); + auto* my_int2 = ty.alias(Source{{56, 78}}, "MyInt", ty.i32()); + AST().AddConstructedType(my_int1); + AST().AddConstructedType(my_int2); + WrapInFunction(); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "56:78 error: type with the name 'MyInt' was already declared\n" + "12:34 note: first declared here"); +} + TEST_F(ResolverTest, Stmt_VariableDecl_ModuleScope) { auto* init = Expr(2); Global("my_var", ty.i32(), ast::StorageClass::kInput, init);