Use the DeclNameStack for impl declarations. (#3691)

They don't have names, but using the DeclNameStack anyway keeps our
behavior more consistent, and keeps track of the enclosing name scope
and the prior state of the scope stack for us.

Depends on #3683.

toolchain/check/decl_name_stack.cpp

@@ -47,6 +47,15 @@ auto DeclNameStack::FinishName() -> NameContext {
   return result;
 }
 
+auto DeclNameStack::FinishImplName() -> NameContext {
+  CARBON_CHECK(decl_name_stack_.back().state == NameContext::State::Empty)
+      << "Impl has a name";
+
+  NameContext result = decl_name_stack_.back();
+  decl_name_stack_.back().state = NameContext::State::Finished;
+  return result;
+}
+
 auto DeclNameStack::PopScope() -> void {
   CARBON_CHECK(decl_name_stack_.back().state == NameContext::State::Finished)
       << "Missing call to FinishName before PopScope";

toolchain/check/decl_name_stack.h

@@ -91,11 +91,15 @@ class DeclNameStack {
       Error,
     };
 
+    // Returns whether the name resolved to an existing entity.
+    auto is_resolved() -> bool {
+      return state != State::Unresolved && state != State::Empty;
+    }
+
     // Returns the name_id for a new instruction. This is invalid when the name
     // resolved.
     auto name_id_for_new_inst() -> SemIR::NameId {
-      return state == State::Unresolved ? unresolved_name_id
-                                        : SemIR::NameId::Invalid;
+      return !is_resolved() ? unresolved_name_id : SemIR::NameId::Invalid;
     }
 
     // Returns the enclosing_scope_id for a new instruction. This is invalid
@@ -103,8 +107,7 @@ class DeclNameStack {
     // the NameContext, which refers to the scope of the introducer rather than
     // the scope of the name.
     auto enclosing_scope_id_for_new_inst() -> SemIR::NameScopeId {
-      return state == State::Unresolved ? target_scope_id
-                                        : SemIR::NameScopeId::Invalid;
+      return !is_resolved() ? target_scope_id : SemIR::NameScopeId::Invalid;
     }
 
     // The current scope when this name began. This is the scope that we will
@@ -141,6 +144,13 @@ 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.
+  auto PeekTargetScope() -> SemIR::NameScopeId {
+    return decl_name_stack_.back().target_scope_id;
+  }
+
   // Finishes the current declaration name processing, returning the final
   // context for adding the name to lookup.
   //
@@ -148,6 +158,13 @@ class DeclNameStack {
   // which will be applied to the declaration name if appropriate.
   auto FinishName() -> NameContext;
 
+  // Finishes the current declaration name processing for an `impl`, returning
+  // the final context for adding the name to lookup.
+  //
+  // `impl`s don't actually have names, but want the rest of the name processing
+  // logic such as building parameter scopes, so are a special case.
+  auto FinishImplName() -> NameContext;
+
   // Pops the declaration name from the declaration name stack, and pops all
   // scopes that were entered as part of handling the declaration name. These
   // are the scopes corresponding to name qualifiers in the name, for example

toolchain/check/handle_impl.cpp

@@ -4,6 +4,7 @@
 
 #include "toolchain/check/context.h"
 #include "toolchain/check/convert.h"
+#include "toolchain/check/decl_name_stack.h"
 #include "toolchain/check/modifiers.h"
 #include "toolchain/sem_ir/ids.h"
 #include "toolchain/sem_ir/typed_insts.h"
@@ -17,15 +18,15 @@ auto HandleImplIntroducer(Context& context, Parse::ImplIntroducerId parse_node)
   context.inst_block_stack().Push();
 
   // Push the bracketing node.
-  context.node_stack().Push(parse_node,
-                            context.scope_stack().PeekNameScopeId());
+  context.node_stack().Push(parse_node);
 
   // Optional modifiers follow.
   context.decl_state_stack().Push(DeclState::Impl);
 
-  // Create a scope for implicit parameters. We may not use it, but it's simpler
-  // to create it unconditionally than to track whether it exists.
-  context.scope_stack().Push();
+  // An impl doesn't have a name per se, but it makes the processing more
+  // consistent to imagine that it does. This also gives us a scope for implicit
+  // parameters.
+  context.decl_name_stack().PushScopeAndStartName();
   return true;
 }
 
@@ -68,8 +69,7 @@ auto HandleDefaultSelfImplAs(Context& context,
   // TODO: Do this without modifying the node stack.
   auto implicit_param_list =
       context.node_stack().PopWithParseNodeIf<Parse::NodeKind::ImplForall>();
-  auto enclosing_scope_id =
-      context.node_stack().Peek<Parse::NodeKind::ImplIntroducer>();
+  auto enclosing_scope_id = context.decl_name_stack().PeekTargetScope();
   if (implicit_param_list) {
     context.node_stack().Push(implicit_param_list->first,
                               implicit_param_list->second);
@@ -90,8 +90,9 @@ auto HandleDefaultSelfImplAs(Context& context,
 // Process an `extend impl` declaration by extending the impl scope with the
 // `impl`'s scope.
 static auto ExtendImpl(Context& context, Parse::AnyImplDeclId parse_node,
-                       SemIR::TypeId constraint_id,
-                       SemIR::NameScopeId enclosing_scope_id) -> void {
+                       SemIR::TypeId constraint_id) -> void {
+  auto enclosing_scope_id = context.decl_name_stack().PeekTargetScope();
+
   // TODO: This is also valid in a mixin.
   if (!TryAsClassScope(context, enclosing_scope_id)) {
     CARBON_DIAGNOSTIC(ExtendImplOutsideClass, Error,
@@ -120,26 +121,17 @@ static auto ExtendImpl(Context& context, Parse::AnyImplDeclId parse_node,
   enclosing_scope.extended_scopes.push_back(interface.scope_id);
 }
 
-namespace {
-struct BuildImplDeclResult {
-  SemIR::ImplId impl_id;
-  SemIR::InstId impl_decl_id;
-  SemIR::NameScopeId enclosing_scope_id;
-};
-}  // namespace
-
 // Build an ImplDecl describing the signature of an impl. This handles the
 // common logic shared by impl forward declarations and impl definitions.
 static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId parse_node)
-    -> BuildImplDeclResult {
+    -> std::pair<SemIR::ImplId, SemIR::InstId> {
   auto [constraint_node, constraint_id] =
       context.node_stack().PopExprWithParseNode();
   auto self_type_id = context.node_stack().Pop<Parse::NodeCategory::ImplAs>();
 
   auto params_id = context.node_stack().PopIf<Parse::NodeKind::ImplForall>();
   auto decl_block_id = context.inst_block_stack().Pop();
-  auto enclosing_scope_id =
-      context.node_stack().Pop<Parse::NodeKind::ImplIntroducer>();
+  context.node_stack().PopForSoloParseNode<Parse::NodeKind::ImplIntroducer>();
 
   // Convert the constraint expression to a type.
   // TODO: Check that its constant value is a constraint.
@@ -151,6 +143,11 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId parse_node)
   LimitModifiersOnDecl(context, KeywordModifierSet::ImplDecl,
                        Lex::TokenKind::Impl);
 
+  // Finish processing the name, which should be empty, but might have
+  // parameters.
+  auto name_context = context.decl_name_stack().FinishImplName();
+  CARBON_CHECK(name_context.state == DeclNameStack::NameContext::State::Empty);
+
   // Add the impl declaration.
   auto impl_decl = SemIR::ImplDecl{SemIR::ImplId::Invalid, decl_block_id};
   auto impl_decl_id = context.AddPlaceholderInst({parse_node, impl_decl});
@@ -172,25 +169,24 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId parse_node)
          KeywordModifierSet::Extend)) {
     // TODO: Diagnose combining `extend` with `forall`.
     // TODO: Diagnose combining `extend` with an explicit self type.
-    ExtendImpl(context, parse_node, constraint_type_id, enclosing_scope_id);
+    ExtendImpl(context, parse_node, constraint_type_id);
   }
 
   context.decl_state_stack().Pop(DeclState::Impl);
 
-  return {impl_decl.impl_id, impl_decl_id, enclosing_scope_id};
+  return {impl_decl.impl_id, impl_decl_id};
 }
 
 auto HandleImplDecl(Context& context, Parse::ImplDeclId parse_node) -> bool {
   BuildImplDecl(context, parse_node);
-  context.scope_stack().Pop();
+  context.decl_name_stack().PopScope();
   return true;
 }
 
 auto HandleImplDefinitionStart(Context& context,
                                Parse::ImplDefinitionStartId parse_node)
     -> bool {
-  auto [impl_id, impl_decl_id, enclosing_scope_id] =
-      BuildImplDecl(context, parse_node);
+  auto [impl_id, impl_decl_id] = BuildImplDecl(context, parse_node);
   auto& impl_info = context.impls().Get(impl_id);
 
   if (impl_info.definition_id.is_valid()) {
@@ -207,8 +203,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, enclosing_scope_id);
+    impl_info.scope_id =
+        context.name_scopes().Add(impl_decl_id, SemIR::NameId::Invalid,
+                                  context.decl_name_stack().PeekTargetScope());
   }
 
   context.scope_stack().Push(impl_decl_id, impl_info.scope_id);
@@ -234,9 +231,7 @@ auto HandleImplDefinition(Context& context,
   auto impl_id =
       context.node_stack().Pop<Parse::NodeKind::ImplDefinitionStart>();
   context.inst_block_stack().Pop();
-  // Pop the impl body scope and the parameter scope.
-  context.scope_stack().Pop();
-  context.scope_stack().Pop();
+  context.decl_name_stack().PopScope();
 
   // The impl is now fully defined.
   context.impls().Get(impl_id).defined = true;

toolchain/check/node_stack.h

@@ -356,7 +356,7 @@ class NodeStack {
   // that the parse node should not appear in the node stack at all.
   using Id = IdUnion<SemIR::InstId, SemIR::InstBlockId, SemIR::FunctionId,
                      SemIR::ClassId, SemIR::InterfaceId, SemIR::ImplId,
-                     SemIR::NameId, SemIR::NameScopeId, SemIR::TypeId>;
+                     SemIR::NameId, SemIR::TypeId>;
 
   // An entry in stack_.
   struct Entry {
@@ -452,8 +452,6 @@ class NodeStack {
           return Id::KindFor<SemIR::ImplId>();
         case Parse::NodeKind::SelfValueName:
           return Id::KindFor<SemIR::NameId>();
-        case Parse::NodeKind::ImplIntroducer:
-          return Id::KindFor<SemIR::NameScopeId>();
         case Parse::NodeKind::ArrayExprSemi:
         case Parse::NodeKind::ClassIntroducer:
         case Parse::NodeKind::CodeBlockStart:
@@ -461,6 +459,7 @@ class NodeStack {
         case Parse::NodeKind::FunctionIntroducer:
         case Parse::NodeKind::IfStatementElse:
         case Parse::NodeKind::ImplicitParamListStart:
+        case Parse::NodeKind::ImplIntroducer:
         case Parse::NodeKind::InterfaceIntroducer:
         case Parse::NodeKind::LetIntroducer:
         case Parse::NodeKind::QualifiedName: