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