Rewrite symbolic constants in generic redeclarations (#5154)

When a generic function declaration was encountered for the second or
more time, we would FinishGenericRedecl() for the function decl, but
this just popped the generic region stack and moved on.

The issue with that is when the stack entry is gone, we lose the
symbolic constants from that declaration, and are unable to rewrite them
to point to the actual generic. This left us with a function declaration
with abstract symbolic values that were not useful, and in a function
call we use the declaration attached to the definition, which would be a
declaration with broken symbolic values. Then the function would be
uncallable since deduce would be unable to determine argument types
without the generic bindings.

This resolves the issue for functions, as well as ensuring the correct
generic id from a previous declaration is used for other generic entity
types that have redeclarations.

When a function declaration is qualified, such as defining a class
method outside the class body, we need only the function declaration to
contribute to its generic region stack. The code was collecting constant
values from all qualifier segments together incorrectly.

So when we PushNameQualifierScope(), we also drop the current generic
region stack and rewrite its constant values by calling
FinishGenericRedecl(), and open a new stack entry for the next part of
the qualified declaration.

If a generic declaration somehow has more dependent instruction than a
previous declaration, it would add new instructions to its eval block
with indices beyond the elements in the actual declaration eval block,
since we only store the block from the first declaration found. To avoid
this we plumb through that we are in a redeclaration, and terminate with
an ICE instead of adding new instructions to crash on later.

Fixes #5136.

toolchain/check/decl_name_stack.cpp

@@ -209,18 +209,30 @@ auto DeclNameStack::LookupOrAddName(NameContext name_context,
 static auto PushNameQualifierScope(Context& context, SemIRLoc loc,
                                    SemIR::InstId scope_inst_id,
                                    SemIR::NameScopeId scope_id,
-                                   SemIR::SpecificId specific_id,
+                                   SemIR::GenericId generic_id,
                                    bool has_error = false) -> void {
   // If the qualifier has no parameters, we don't need to keep around a
   // parameter scope.
   context.scope_stack().PopIfEmpty();
 
-  // When declaring a member of a generic, resolve the self specific.
-  if (specific_id.has_value()) {
-    ResolveSpecificDefinition(context, loc, specific_id);
+  auto self_specific_id = SemIR::SpecificId::None;
+  if (generic_id.has_value()) {
+    self_specific_id = context.generics().GetSelfSpecific(generic_id);
+    // When declaring a member of a generic, resolve the self specific.
+    ResolveSpecificDefinition(context, loc, self_specific_id);
   }
 
-  context.scope_stack().Push(scope_inst_id, scope_id, specific_id, has_error);
+  // Close the generic stack scope and open a new one for whatever comes after
+  // the qualifier. As this is a qualifier it must not be the initial
+  // declaration of the entity, so we treat it as a redeclaration.
+  FinishGenericRedecl(context, generic_id);
+  // What follows the qualifier will be a declaration. The signature of an
+  // entity is also a declaration even if it is followed by curly braces
+  // providing the definition.
+  StartGenericDecl(context);
+
+  context.scope_stack().Push(scope_inst_id, scope_id, self_specific_id,
+                             has_error);
 
   // An interface also introduces its 'Self' parameter into scope, despite it
   // not being redeclared as part of the qualifier.
@@ -241,10 +253,10 @@ auto DeclNameStack::ApplyNameQualifier(const NameComponent& name) -> void {
   name_context.has_qualifiers = true;
 
   // Resolve the qualifier as a scope and enter the new scope.
-  auto [scope_id, specific_id] = ResolveAsScope(name_context, name);
+  auto [scope_id, generic_id] = ResolveAsScope(name_context, name);
   if (scope_id.has_value()) {
     PushNameQualifierScope(*context_, name_context.loc_id,
-                           name_context.resolved_inst_id, scope_id, specific_id,
+                           name_context.resolved_inst_id, scope_id, generic_id,
                            context_->name_scopes().Get(scope_id).has_error());
     name_context.parent_scope_id = scope_id;
   } else {
@@ -374,9 +386,9 @@ static auto DiagnoseQualifiedDeclInNonScope(Context& context, SemIRLoc use_loc,
 
 auto DeclNameStack::ResolveAsScope(const NameContext& name_context,
                                    const NameComponent& name) const
-    -> std::pair<SemIR::NameScopeId, SemIR::SpecificId> {
-  constexpr std::pair<SemIR::NameScopeId, SemIR::SpecificId> InvalidResult = {
-      SemIR::NameScopeId::None, SemIR::SpecificId::None};
+    -> std::pair<SemIR::NameScopeId, SemIR::GenericId> {
+  constexpr std::pair<SemIR::NameScopeId, SemIR::GenericId> InvalidResult = {
+      SemIR::NameScopeId::None, SemIR::GenericId::None};
 
   if (!CheckQualifierIsResolved(*context_, name_context)) {
     return InvalidResult;
@@ -406,8 +418,7 @@ auto DeclNameStack::ResolveAsScope(const NameContext& name_context,
             *context_, name_context.loc_id, class_decl.class_id);
         return InvalidResult;
       }
-      return {class_info.scope_id,
-              context_->generics().GetSelfSpecific(class_info.generic_id)};
+      return {class_info.scope_id, class_info.generic_id};
     }
     case CARBON_KIND(SemIR::InterfaceDecl interface_decl): {
       const auto& interface_info =
@@ -422,8 +433,7 @@ auto DeclNameStack::ResolveAsScope(const NameContext& name_context,
             name_context.resolved_inst_id);
         return InvalidResult;
       }
-      return {interface_info.scope_id,
-              context_->generics().GetSelfSpecific(interface_info.generic_id)};
+      return {interface_info.scope_id, interface_info.generic_id};
     }
     case CARBON_KIND(SemIR::Namespace resolved_inst): {
       auto scope_id = resolved_inst.name_scope_id;
@@ -443,7 +453,7 @@ auto DeclNameStack::ResolveAsScope(const NameContext& name_context,
         // be used as a name qualifier.
         scope.set_is_closed_import(false);
       }
-      return {scope_id, SemIR::SpecificId::None};
+      return {scope_id, SemIR::GenericId::None};
     }
     default: {
       DiagnoseQualifiedDeclInNonScope(*context_, name_context.loc_id,