On NameContext, rename enclosing_scope and target_scope_id. (#3948)

I think the new name is more consistent for how `enclosing_scope_id` is
used relative to `name_id` (even removing the clarifying note on
`enclosing_scope_id_for_new_inst`). Suggesting `initial_scope_index` as
a replacing for the old `enclosing_scope`, hoping it's a little clearer.

I'm replacing `target_scope_id` uses in modifier logic because they
seemed to be based on the NameContext use.

toolchain/check/check.cpp

@@ -488,7 +488,7 @@ class DeferredDefinitionWorklist {
   auto PushEnterDeferredDefinitionScope(Context& context) -> void {
     bool nested = !enclosing_scopes_.empty() &&
                   enclosing_scopes_.back().scope_index ==
-                      context.decl_name_stack().PeekEnclosingScope();
+                      context.decl_name_stack().PeekInitialScopeIndex();
     enclosing_scopes_.push_back(
         {.worklist_start_index = worklist_.size(),
          .scope_index = context.scope_stack().PeekIndex()});

toolchain/check/decl_name_stack.cpp

@@ -35,8 +35,8 @@ auto DeclNameStack::NameContext::prev_inst_id() -> SemIR::InstId {
 
 auto DeclNameStack::MakeEmptyNameContext() -> NameContext {
   return NameContext{
-      .enclosing_scope = context_->scope_stack().PeekIndex(),
-      .target_scope_id = context_->scope_stack().PeekNameScopeId()};
+      .initial_scope_index = context_->scope_stack().PeekIndex(),
+      .enclosing_scope_id = context_->scope_stack().PeekNameScopeId()};
 }
 
 auto DeclNameStack::MakeUnqualifiedName(SemIR::LocId loc_id,
@@ -83,7 +83,7 @@ auto DeclNameStack::FinishImplName() -> NameContext {
 auto DeclNameStack::PopScope() -> void {
   CARBON_CHECK(decl_name_stack_.back().state == NameContext::State::Finished)
       << "Missing call to FinishName before PopScope";
-  context_->scope_stack().PopTo(decl_name_stack_.back().enclosing_scope);
+  context_->scope_stack().PopTo(decl_name_stack_.back().initial_scope_index);
   decl_name_stack_.pop_back();
 }
 
@@ -91,7 +91,7 @@ auto DeclNameStack::Suspend() -> SuspendedName {
   CARBON_CHECK(decl_name_stack_.back().state == NameContext::State::Finished)
       << "Missing call to FinishName before Suspend";
   SuspendedName result = {decl_name_stack_.pop_back_val(), {}};
-  auto enclosing_index = result.name_context.enclosing_scope;
+  auto enclosing_index = result.name_context.initial_scope_index;
   auto& scope_stack = context_->scope_stack();
   while (scope_stack.PeekIndex() > enclosing_index) {
     result.scopes.push_back(scope_stack.Suspend());
@@ -105,7 +105,7 @@ auto DeclNameStack::Suspend() -> SuspendedName {
 auto DeclNameStack::Restore(SuspendedName sus) -> void {
   // The enclosing state must be the same when a name is restored.
   CARBON_CHECK(context_->scope_stack().PeekIndex() ==
-               sus.name_context.enclosing_scope)
+               sus.name_context.initial_scope_index)
       << "Name restored at the wrong position in the name stack.";
 
   // clang-tidy warns that the `std::move` below has no effect. While that's
@@ -124,11 +124,11 @@ auto DeclNameStack::AddName(NameContext name_context, SemIR::InstId target_id)
       return;
 
     case NameContext::State::Unresolved:
-      if (!name_context.target_scope_id.is_valid()) {
+      if (!name_context.enclosing_scope_id.is_valid()) {
         context_->AddNameToLookup(name_context.unresolved_name_id, target_id);
       } else {
         auto& name_scope =
-            context_->name_scopes().Get(name_context.target_scope_id);
+            context_->name_scopes().Get(name_context.enclosing_scope_id);
         if (name_context.has_qualifiers) {
           auto inst = context_->insts().Get(name_scope.inst_id);
           if (!inst.Is<SemIR::Namespace>()) {
@@ -144,7 +144,7 @@ auto DeclNameStack::AddName(NameContext name_context, SemIR::InstId target_id)
 
         // Exports are only tracked when the declaration is at the file-level
         // scope. Otherwise, it's in some other entity, such as a class.
-        if (name_context.enclosing_scope == ScopeIndex::Package) {
+        if (name_context.initial_scope_index == ScopeIndex::Package) {
           context_->AddExport(target_id);
         }
 
@@ -153,7 +153,7 @@ auto DeclNameStack::AddName(NameContext name_context, SemIR::InstId target_id)
         CARBON_CHECK(success)
             << "Duplicate names should have been resolved previously: "
             << name_context.unresolved_name_id << " in "
-            << name_context.target_scope_id;
+            << name_context.enclosing_scope_id;
       }
       break;
 
@@ -195,11 +195,11 @@ auto DeclNameStack::ApplyNameQualifierTo(NameContext& name_context,
   if (TryResolveQualifier(name_context, loc_id)) {
     // For identifier nodes, we need to perform a lookup on the identifier.
     auto resolved_inst_id = context_->LookupNameInDecl(
-        name_context.loc_id, name_id, name_context.target_scope_id);
+        name_context.loc_id, name_id, name_context.enclosing_scope_id);
     if (!resolved_inst_id.is_valid()) {
       // Invalid indicates an unresolved name. Store it and return.
-      name_context.state = NameContext::State::Unresolved;
       name_context.unresolved_name_id = name_id;
+      name_context.state = NameContext::State::Unresolved;
       return;
     } else {
       // Store the resolved instruction and continue for the target scope
@@ -240,7 +240,7 @@ auto DeclNameStack::UpdateScopeIfNeeded(NameContext& name_context,
       const auto& class_info = context_->classes().Get(resolved_inst.class_id);
       if (class_info.is_defined()) {
         name_context.state = NameContext::State::Resolved;
-        name_context.target_scope_id = class_info.scope_id;
+        name_context.enclosing_scope_id = class_info.scope_id;
         if (!is_unqualified) {
           PushNameQualifierScope(*context_, name_context.resolved_inst_id,
                                  class_info.scope_id);
@@ -255,7 +255,7 @@ auto DeclNameStack::UpdateScopeIfNeeded(NameContext& name_context,
           context_->interfaces().Get(resolved_inst.interface_id);
       if (interface_info.is_defined()) {
         name_context.state = NameContext::State::Resolved;
-        name_context.target_scope_id = interface_info.scope_id;
+        name_context.enclosing_scope_id = interface_info.scope_id;
         if (!is_unqualified) {
           PushNameQualifierScope(*context_, name_context.resolved_inst_id,
                                  interface_info.scope_id);
@@ -268,7 +268,7 @@ auto DeclNameStack::UpdateScopeIfNeeded(NameContext& name_context,
     case CARBON_KIND(SemIR::Namespace resolved_inst): {
       auto scope_id = resolved_inst.name_scope_id;
       name_context.state = NameContext::State::Resolved;
-      name_context.target_scope_id = scope_id;
+      name_context.enclosing_scope_id = scope_id;
       auto& scope = context_->name_scopes().Get(scope_id);
       if (scope.is_closed_import) {
         CARBON_DIAGNOSTIC(QualifiedDeclOutsidePackage, Error,

toolchain/check/decl_name_stack.h

@@ -102,17 +102,15 @@ class DeclNameStack {
     }
 
     // Returns the enclosing_scope_id for a new instruction. This is invalid
-    // when the name resolved. Note this is distinct from the enclosing_scope of
-    // the NameContext, which refers to the scope of the introducer rather than
-    // the scope of the name.
+    // when the name resolved.
     auto enclosing_scope_id_for_new_inst() -> SemIR::NameScopeId {
-      return state == State::Unresolved ? target_scope_id
+      return state == State::Unresolved ? enclosing_scope_id
                                         : SemIR::NameScopeId::Invalid;
     }
 
     // The current scope when this name began. This is the scope that we will
     // return to at the end of the declaration.
-    ScopeIndex enclosing_scope;
+    ScopeIndex initial_scope_index;
 
     State state = State::Empty;
 
@@ -122,7 +120,7 @@ class DeclNameStack {
     // The scope which qualified names are added to. For unqualified names in
     // an unnamed scope, this will be Invalid to indicate the current scope
     // should be used.
-    SemIR::NameScopeId target_scope_id;
+    SemIR::NameScopeId enclosing_scope_id;
 
     // The last location ID used.
     SemIR::LocId loc_id = SemIR::LocId::Invalid;
@@ -159,18 +157,18 @@ class DeclNameStack {
   // state, `FinishName` and `PopScope` must be called, in that order.
   auto PushScopeAndStartName() -> void;
 
-  // Peeks the current target scope of the name on top of the stack. Note that
-  // if we're still processing the name qualifiers, this can change before the
-  // name is completed. Also, if the name up to this point was already declared
-  // and is a scope, this will be that scope, rather than the scope enclosing
-  // it.
-  auto PeekTargetScope() const -> SemIR::NameScopeId {
-    return decl_name_stack_.back().target_scope_id;
+  // Peeks the current enclosing scope of the name on top of the stack. Note
+  // that if we're still processing the name qualifiers, this can change before
+  // the name is completed. Also, if the name up to this point was already
+  // declared and is a scope, this will be that scope, rather than the scope
+  // enclosing it.
+  auto PeekEnclosingScopeId() const -> SemIR::NameScopeId {
+    return decl_name_stack_.back().enclosing_scope_id;
   }
 
-  // Peeks the enclosing scope index of the name on top of the stack.
-  auto PeekEnclosingScope() const -> ScopeIndex {
-    return decl_name_stack_.back().enclosing_scope;
+  // Peeks the resolution scope index of the name on top of the stack.
+  auto PeekInitialScopeIndex() const -> ScopeIndex {
+    return decl_name_stack_.back().initial_scope_index;
   }
 
   // Finishes the current declaration name processing, returning the final

toolchain/check/handle_class.cpp

@@ -189,13 +189,13 @@ static auto BuildClassDecl(Context& context, Parse::AnyClassDeclId node_id,
 
   // Process modifiers.
   CheckAccessModifiersOnDecl(context, Lex::TokenKind::Class,
-                             name_context.target_scope_id);
+                             name_context.enclosing_scope_id);
   LimitModifiersOnDecl(context,
                        KeywordModifierSet::Class | KeywordModifierSet::Access |
                            KeywordModifierSet::Extern,
                        Lex::TokenKind::Class);
   RestrictExternModifierOnDecl(context, Lex::TokenKind::Class,
-                               name_context.target_scope_id, is_definition);
+                               name_context.enclosing_scope_id, is_definition);
 
   auto modifiers = context.decl_state_stack().innermost().modifier_set;
   if (!!(modifiers & KeywordModifierSet::Access)) {

toolchain/check/handle_function.cpp

@@ -44,19 +44,19 @@ auto HandleReturnType(Context& context, Parse::ReturnTypeId node_id) -> bool {
 }
 
 static auto DiagnoseModifiers(Context& context, bool is_definition,
-                              SemIR::NameScopeId target_scope_id)
+                              SemIR::NameScopeId enclosing_scope_id)
     -> KeywordModifierSet {
   const Lex::TokenKind decl_kind = Lex::TokenKind::Fn;
-  CheckAccessModifiersOnDecl(context, decl_kind, target_scope_id);
+  CheckAccessModifiersOnDecl(context, decl_kind, enclosing_scope_id);
   LimitModifiersOnDecl(context,
                        KeywordModifierSet::Access | KeywordModifierSet::Extern |
                            KeywordModifierSet::Method |
                            KeywordModifierSet::Interface,
                        decl_kind);
-  RestrictExternModifierOnDecl(context, decl_kind, target_scope_id,
+  RestrictExternModifierOnDecl(context, decl_kind, enclosing_scope_id,
                                is_definition);
-  CheckMethodModifiersOnFunction(context, target_scope_id);
-  RequireDefaultFinalOnlyInInterfaces(context, decl_kind, target_scope_id);
+  CheckMethodModifiersOnFunction(context, enclosing_scope_id);
+  RequireDefaultFinalOnlyInInterfaces(context, decl_kind, enclosing_scope_id);
 
   return context.decl_state_stack().innermost().modifier_set;
 }
@@ -223,8 +223,8 @@ static auto BuildFunctionDecl(Context& context,
       .PopAndDiscardSoloNodeId<Parse::NodeKind::FunctionIntroducer>();
 
   // Process modifiers.
-  auto modifiers =
-      DiagnoseModifiers(context, is_definition, name_context.target_scope_id);
+  auto modifiers = DiagnoseModifiers(context, is_definition,
+                                     name_context.enclosing_scope_id);
   if (!!(modifiers & KeywordModifierSet::Access)) {
     context.TODO(context.decl_state_stack().innermost().modifier_node_id(
                      ModifierOrder::Access),

toolchain/check/handle_impl.cpp

@@ -69,7 +69,7 @@ static auto TryAsClassScope(Context& context, SemIR::NameScopeId scope_id)
 }
 
 static auto GetDefaultSelfType(Context& context) -> SemIR::TypeId {
-  auto enclosing_scope_id = context.decl_name_stack().PeekTargetScope();
+  auto enclosing_scope_id = context.decl_name_stack().PeekEnclosingScopeId();
 
   if (auto class_decl = TryAsClassScope(context, enclosing_scope_id)) {
     return context.classes().Get(class_decl->class_id).self_type_id;
@@ -103,7 +103,7 @@ static auto ExtendImpl(Context& context, Parse::NodeId extend_node,
                        Parse::NodeId self_type_node, SemIR::TypeId self_type_id,
                        Parse::NodeId params_node, SemIR::TypeId constraint_id)
     -> void {
-  auto enclosing_scope_id = context.decl_name_stack().PeekTargetScope();
+  auto enclosing_scope_id = context.decl_name_stack().PeekEnclosingScopeId();
   auto& enclosing_scope = context.name_scopes().Get(enclosing_scope_id);
 
   // TODO: This is also valid in a mixin.
@@ -249,9 +249,9 @@ auto HandleImplDefinitionStart(Context& context,
         .Emit();
   } else {
     impl_info.definition_id = impl_decl_id;
-    impl_info.scope_id =
-        context.name_scopes().Add(impl_decl_id, SemIR::NameId::Invalid,
-                                  context.decl_name_stack().PeekTargetScope());
+    impl_info.scope_id = context.name_scopes().Add(
+        impl_decl_id, SemIR::NameId::Invalid,
+        context.decl_name_stack().PeekEnclosingScopeId());
   }
 
   context.scope_stack().Push(impl_decl_id, impl_info.scope_id);

toolchain/check/handle_interface.cpp

@@ -38,7 +38,7 @@ static auto BuildInterfaceDecl(Context& context,
 
   // Process modifiers.
   CheckAccessModifiersOnDecl(context, Lex::TokenKind::Interface,
-                             name_context.target_scope_id);
+                             name_context.enclosing_scope_id);
   LimitModifiersOnDecl(context, KeywordModifierSet::Access,
                        Lex::TokenKind::Interface);
 

toolchain/check/modifiers.cpp

@@ -83,8 +83,8 @@ static auto GetScopeInst(Context& context, SemIR::NameScopeId scope_id)
 }
 
 auto CheckAccessModifiersOnDecl(Context& context, Lex::TokenKind decl_kind,
-                                SemIR::NameScopeId target_scope_id) -> void {
-  auto target = GetScopeInst(context, target_scope_id);
+                                SemIR::NameScopeId enclosing_scope_id) -> void {
+  auto target = GetScopeInst(context, enclosing_scope_id);
   if (target && target->Is<SemIR::Namespace>()) {
     // TODO: This assumes that namespaces can only be declared at file scope. If
     // we add support for non-file-scope namespaces, we will need to check the
@@ -109,10 +109,10 @@ auto CheckAccessModifiersOnDecl(Context& context, Lex::TokenKind decl_kind,
 }
 
 auto CheckMethodModifiersOnFunction(Context& context,
-                                    SemIR::NameScopeId target_scope_id)
+                                    SemIR::NameScopeId enclosing_scope_id)
     -> void {
   const Lex::TokenKind decl_kind = Lex::TokenKind::Fn;
-  auto target_id = GetScopeInstId(context, target_scope_id);
+  auto target_id = GetScopeInstId(context, enclosing_scope_id);
   if (target_id.is_valid()) {
     if (auto class_decl =
             context.insts().TryGetAs<SemIR::ClassDecl>(target_id)) {
@@ -137,14 +137,14 @@ auto CheckMethodModifiersOnFunction(Context& context,
 }
 
 auto RestrictExternModifierOnDecl(Context& context, Lex::TokenKind decl_kind,
-                                  SemIR::NameScopeId target_scope_id,
+                                  SemIR::NameScopeId enclosing_scope_id,
                                   bool is_definition) -> void {
   if (is_definition) {
     ForbidModifiersOnDecl(context, KeywordModifierSet::Extern, decl_kind,
                           " that provides a definition");
   }
-  if (target_scope_id.is_valid()) {
-    auto target_id = context.name_scopes().Get(target_scope_id).inst_id;
+  if (enclosing_scope_id.is_valid()) {
+    auto target_id = context.name_scopes().Get(enclosing_scope_id).inst_id;
     if (target_id.is_valid() &&
         !context.insts().Is<SemIR::Namespace>(target_id)) {
       ForbidModifiersOnDecl(context, KeywordModifierSet::Extern, decl_kind,
@@ -155,9 +155,9 @@ auto RestrictExternModifierOnDecl(Context& context, Lex::TokenKind decl_kind,
 
 auto RequireDefaultFinalOnlyInInterfaces(Context& context,
                                          Lex::TokenKind decl_kind,
-                                         SemIR::NameScopeId target_scope_id)
+                                         SemIR::NameScopeId enclosing_scope_id)
     -> void {
-  auto target = GetScopeInst(context, target_scope_id);
+  auto target = GetScopeInst(context, enclosing_scope_id);
   if (target && target->Is<SemIR::InterfaceDecl>()) {
     // Both `default` and `final` allowed in an interface definition.
     return;

toolchain/check/modifiers.h

@@ -10,20 +10,21 @@
 namespace Carbon::Check {
 
 // Reports a diagnostic if access control modifiers on this are not allowed for
-// a declaration in `target_scope_id`, and updates the declaration state in
+// a declaration in `enclosing_scope_id`, and updates the declaration state in
 // `context`.
 //
-// `target_scope_id` may be Invalid for a declaration in a block scope.
+// `enclosing_scope_id` may be Invalid for a declaration in a block scope.
 auto CheckAccessModifiersOnDecl(Context& context, Lex::TokenKind decl_kind,
-                                SemIR::NameScopeId target_scope_id) -> void;
+                                SemIR::NameScopeId enclosing_scope_id) -> void;
 
 // Reports a diagnostic if the method function modifiers `abstract`, `virtual`,
 // or `impl` are present but not permitted on a function declaration in
-// `target_scope_id`.
+// `enclosing_scope_id`.
 //
-// `target_scope_id` may be Invalid for a declaration in a block scope.
+// `enclosing_scope_id` may be Invalid for a declaration in a block scope.
 auto CheckMethodModifiersOnFunction(Context& context,
-                                    SemIR::NameScopeId target_scope_id) -> void;
+                                    SemIR::NameScopeId enclosing_scope_id)
+    -> void;
 
 // Like `LimitModifiersOnDecl`, except says which modifiers are forbidden, and a
 // `context_string` (and optional `context_loc_id`) specifying the context in
@@ -48,17 +49,17 @@ inline auto LimitModifiersOnDecl(Context& context, KeywordModifierSet allowed,
 // - `extern` on a definition.
 // - `extern` on a scoped entity.
 auto RestrictExternModifierOnDecl(Context& context, Lex::TokenKind decl_kind,
-                                  SemIR::NameScopeId target_scope_id,
+                                  SemIR::NameScopeId enclosing_scope_id,
                                   bool is_definition) -> void;
 
 // Report a diagonostic if `default` and `final` modifiers are used on
 // declarations where they are not allowed. Right now they are only allowed
 // inside interfaces.
 //
-// `target_scope_id` may be Invalid for a declaration in a block scope.
+// `enclosing_scope_id` may be Invalid for a declaration in a block scope.
 auto RequireDefaultFinalOnlyInInterfaces(Context& context,
                                          Lex::TokenKind decl_kind,
-                                         SemIR::NameScopeId target_scope_id)
+                                         SemIR::NameScopeId enclosing_scope_id)
     -> void;
 
 }  // namespace Carbon::Check