From ccc6498993f5a9e65ffef4a2276e84024dcb7f35 Mon Sep 17 00:00:00 2001 From: pk19604014 <95385881+pk19604014@users.noreply.github.com> Date: Thu, 2 Jun 2022 15:47:41 -0400 Subject: [PATCH] Add alias name to name resolution after processing the target to avoid self-referencing name crash (#1295) Fixes #1294 --- explorer/ast/static_scope.cpp | 32 ++++++++++++++----- explorer/ast/static_scope.h | 14 ++++++-- explorer/interpreter/resolve_names.cpp | 8 +++-- .../testdata/alias/fail_self_alias.carbon | 18 +++++++++++ 4 files changed, 59 insertions(+), 13 deletions(-) create mode 100644 explorer/testdata/alias/fail_self_alias.carbon diff --git a/explorer/ast/static_scope.cpp b/explorer/ast/static_scope.cpp index e217cf7a8e07..d34e94a83445 100644 --- a/explorer/ast/static_scope.cpp +++ b/explorer/ast/static_scope.cpp @@ -9,17 +9,28 @@ namespace Carbon { -auto StaticScope::Add(std::string name, ValueNodeView entity) - -> ErrorOr { - auto [it, success] = declared_names_.insert({name, entity}); - if (!success && it->second != entity) { - return CompilationError(entity.base().source_loc()) - << "Duplicate name `" << name << "` also found at " - << it->second.base().source_loc(); +auto StaticScope::Add(const std::string& name, ValueNodeView entity, + bool usable) -> ErrorOr { + auto [it, inserted] = declared_names_.insert({name, {entity, usable}}); + if (!inserted) { + if (it->second.entity != entity) { + return CompilationError(entity.base().source_loc()) + << "Duplicate name `" << name << "` also found at " + << it->second.entity.base().source_loc(); + } + CARBON_CHECK(usable || !it->second.usable) + << entity.base().source_loc() << " attempting to mark a usable name `" + << name << "` as unusable"; } return Success(); } +void StaticScope::MarkUsable(const std::string& name) { + auto it = declared_names_.find(name); + CARBON_CHECK(it != declared_names_.end()) << name << " not found"; + it->second.usable = true; +} + auto StaticScope::Resolve(const std::string& name, SourceLocation source_loc) const -> ErrorOr { @@ -36,7 +47,12 @@ auto StaticScope::TryResolve(const std::string& name, -> ErrorOr> { auto it = declared_names_.find(name); if (it != declared_names_.end()) { - return std::make_optional(it->second); + if (!it->second.usable) { + return CompilationError(source_loc) + << "'" << name + << "' is not usable until after it has been completely declared"; + } + return std::make_optional(it->second.entity); } std::optional result; for (Nonnull parent : parent_scopes_) { diff --git a/explorer/ast/static_scope.h b/explorer/ast/static_scope.h index b4f6b8bbc068..dd7777306f28 100644 --- a/explorer/ast/static_scope.h +++ b/explorer/ast/static_scope.h @@ -153,7 +153,13 @@ class StaticScope { public: // Defines `name` to be `entity` in this scope, or reports a compilation error // if `name` is already defined to be a different entity in this scope. - auto Add(std::string name, ValueNodeView entity) -> ErrorOr; + // If `usable` is `false`, `name` cannot yet be referenced and `Resolve()` + // methods will fail for it. + auto Add(const std::string& name, ValueNodeView entity, bool usable = true) + -> ErrorOr; + + // Marks `name` as usable. + void MarkUsable(const std::string& name); // Make `parent` a parent of this scope. // REQUIRES: `parent` is not already a parent of this scope. @@ -174,8 +180,12 @@ class StaticScope { auto TryResolve(const std::string& name, SourceLocation source_loc) const -> ErrorOr>; + struct Entry { + ValueNodeView entity; + bool usable = false; + }; // Maps locally declared names to their entities. - std::unordered_map declared_names_; + std::unordered_map declared_names_; // A list of scopes used for name lookup within this scope. std::vector> parent_scopes_; diff --git a/explorer/interpreter/resolve_names.cpp b/explorer/interpreter/resolve_names.cpp index 49c90c906ae7..7399c8e610af 100644 --- a/explorer/interpreter/resolve_names.cpp +++ b/explorer/interpreter/resolve_names.cpp @@ -66,7 +66,8 @@ static auto AddExposedNames(const Declaration& declaration, } case DeclarationKind::AliasDeclaration: { auto& alias = cast(declaration); - CARBON_RETURN_IF_ERROR(enclosing_scope.Add(alias.name(), &alias)); + CARBON_RETURN_IF_ERROR( + enclosing_scope.Add(alias.name(), &alias, /*usable=*/false)); break; } } @@ -458,8 +459,9 @@ static auto ResolveNames(Declaration& declaration, StaticScope& enclosing_scope) } case DeclarationKind::AliasDeclaration: { - CARBON_RETURN_IF_ERROR(ResolveNames( - cast(declaration).target(), enclosing_scope)); + auto& alias = cast(declaration); + CARBON_RETURN_IF_ERROR(ResolveNames(alias.target(), enclosing_scope)); + enclosing_scope.MarkUsable(alias.name()); break; } } diff --git a/explorer/testdata/alias/fail_self_alias.carbon b/explorer/testdata/alias/fail_self_alias.carbon new file mode 100644 index 000000000000..a04eb5afaf1c --- /dev/null +++ b/explorer/testdata/alias/fail_self_alias.carbon @@ -0,0 +1,18 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// RUN: %{not} %{explorer} %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes=false %s +// RUN: %{not} %{explorer} --parser_debug --trace_file=- %s 2>&1 | \ +// RUN: %{FileCheck} --match-full-lines --allow-unused-prefixes %s +// AUTOUPDATE: %{explorer} %s + +package ExplorerTest api; + +// CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/alias/fail_self_alias.carbon:[[@LINE+1]]: 'a' is not usable until after it has been completely declared +alias a = a; + +fn Main() -> i32 { + return 0; +}