Fix a bug importing declarations in generics

toolchain/check/import_ref.cpp

@@ -187,22 +187,27 @@ class ImportRefResolver;
 struct ResolveResult {
   // The new constant value, if known.
   SemIR::ConstantId const_id;
+  // Newly created declaration whose value is being resolved, if any.
+  SemIR::InstId decl_id = SemIR::InstId::Invalid;
   // Whether resolution has been attempted once and needs to be retried.
   bool retry = false;
 
   // Produces a resolve result that tries resolving this instruction again. If
   // `const_id` is specified, then this is the end of the second phase, and the
   // constant value will be passed to the next resolution attempt. Otherwise,
   // this is the end of the first phase.
-  static auto Retry(SemIR::ConstantId const_id = SemIR::ConstantId::Invalid)
+  static auto Retry(SemIR::ConstantId const_id = SemIR::ConstantId::Invalid,
+                    SemIR::InstId decl_id = SemIR::InstId::Invalid)
       -> ResolveResult {
-    return {.const_id = const_id, .retry = true};
+    return {.const_id = const_id, .decl_id = decl_id, .retry = true};
   }
 
   // Produces a resolve result that provides the given constant value. Requires
   // that there is no new work.
-  static auto Done(SemIR::ConstantId const_id) -> ResolveResult {
-    return {.const_id = const_id};
+  static auto Done(SemIR::ConstantId const_id,
+                   SemIR::InstId decl_id = SemIR::InstId::Invalid)
+      -> ResolveResult {
+    return {.const_id = const_id, .decl_id = decl_id};
   }
 };
 }  // namespace
@@ -455,7 +460,7 @@ class ImportRefResolver : public ImportContext {
 
       // Step 2: resolve the instruction.
       initial_work_ = work_stack_.size();
-      auto [new_const_id, retry] =
+      auto [new_const_id, _, retry] =
           TryResolveInst(*this, work.inst_id, existing.const_id);
       CARBON_CHECK(!HasNewWork() || retry);
 
@@ -1309,7 +1314,8 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
       resolver.local_context().MakeImportedLocAndInst<SemIR::AdaptDecl>(
           AddImportIRInst(resolver, import_inst_id),
           {.adapted_type_inst_id = adapted_type_inst_id}));
-  return ResolveResult::Done(resolver.local_constant_values().Get(inst_id));
+  return ResolveResult::Done(resolver.local_constant_values().Get(inst_id),
+                             inst_id);
 }
 
 static auto TryResolveTypedInst(ImportRefResolver& resolver,
@@ -1389,7 +1395,8 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
                resolver.local_context().GetTypeIdForTypeConstant(type_const_id),
            .base_type_inst_id = base_type_inst_id,
            .index = inst.index}));
-  return ResolveResult::Done(resolver.local_constant_values().Get(inst_id));
+  return ResolveResult::Done(resolver.local_constant_values().Get(inst_id),
+                             inst_id);
 }
 
 static auto TryResolveTypedInst(ImportRefResolver& resolver,
@@ -1602,12 +1609,12 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
   auto adapt_id = import_class.adapt_id.is_valid()
                       ? GetLocalConstantInstId(resolver, import_class.adapt_id)
                       : SemIR::InstId::Invalid;
+  auto& new_class = resolver.local_classes().Get(class_id);
 
   if (resolver.HasNewWork()) {
-    return ResolveResult::Retry(class_const_id);
+    return ResolveResult::Retry(class_const_id, new_class.first_decl_id());
   }
 
-  auto& new_class = resolver.local_classes().Get(class_id);
   new_class.parent_scope_id = parent_scope_id;
   new_class.implicit_param_patterns_id = GetLocalParamPatternsId(
       resolver, import_class.implicit_param_patterns_id);
@@ -1628,7 +1635,7 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
                        complete_type_witness_id, base_id, adapt_id);
   }
 
-  return ResolveResult::Done(class_const_id);
+  return ResolveResult::Done(class_const_id, new_class.first_decl_id());
 }
 
 static auto TryResolveTypedInst(ImportRefResolver& resolver,
@@ -1713,7 +1720,8 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
                resolver.local_context().GetTypeIdForTypeConstant(const_id),
            .name_id = GetLocalNameId(resolver, inst.name_id),
            .index = inst.index}));
-  return {.const_id = resolver.local_constant_values().Get(inst_id)};
+  return ResolveResult::Done(resolver.local_constant_values().Get(inst_id),
+                             inst_id);
 }
 
 // Make a declaration of a function. This is done as a separate step from
@@ -1794,13 +1802,14 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
                               import_function.implicit_param_patterns_id);
   LoadLocalPatternConstantIds(resolver, import_function.param_patterns_id);
   auto generic_data = GetLocalGenericData(resolver, import_function.generic_id);
+  auto& new_function = resolver.local_functions().Get(function_id);
 
   if (resolver.HasNewWork()) {
-    return ResolveResult::Retry(function_const_id);
+    return ResolveResult::Retry(function_const_id,
+                                new_function.first_decl_id());
   }
 
   // Add the function declaration.
-  auto& new_function = resolver.local_functions().Get(function_id);
   new_function.parent_scope_id = parent_scope_id;
   new_function.implicit_param_patterns_id = GetLocalParamPatternsId(
       resolver, import_function.implicit_param_patterns_id);
@@ -1815,7 +1824,7 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
     new_function.definition_id = new_function.first_owning_decl_id;
   }
 
-  return ResolveResult::Done(function_const_id);
+  return ResolveResult::Done(function_const_id, new_function.first_decl_id());
 }
 
 static auto TryResolveTypedInst(ImportRefResolver& resolver,
@@ -1963,12 +1972,12 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
   auto constraint_const_id = GetLocalConstantId(
       resolver,
       resolver.import_constant_values().Get(import_impl.constraint_id));
+  auto& new_impl = resolver.local_impls().Get(impl_id);
 
   if (resolver.HasNewWork()) {
-    return ResolveResult::Retry(impl_const_id);
+    return ResolveResult::Retry(impl_const_id, new_impl.first_decl_id());
   }
 
-  auto& new_impl = resolver.local_impls().Get(impl_id);
   new_impl.parent_scope_id = parent_scope_id;
   new_impl.implicit_param_patterns_id =
       GetLocalParamPatternsId(resolver, import_impl.implicit_param_patterns_id);
@@ -1995,7 +2004,7 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
     resolver.local_impls().GetOrAddLookupBucket(new_impl).push_back(impl_id);
   }
 
-  return ResolveResult::Done(impl_const_id);
+  return ResolveResult::Done(impl_const_id, new_impl.first_decl_id());
 }
 
 static auto TryResolveTypedInst(ImportRefResolver& resolver,
@@ -2135,12 +2144,13 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
     self_param_id =
         GetLocalConstantInstId(resolver, import_interface.self_param_id);
   }
+  auto& new_interface = resolver.local_interfaces().Get(interface_id);
 
   if (resolver.HasNewWork()) {
-    return ResolveResult::Retry(interface_const_id);
+    return ResolveResult::Retry(interface_const_id,
+                                new_interface.first_decl_id());
   }
 
-  auto& new_interface = resolver.local_interfaces().Get(interface_id);
   new_interface.parent_scope_id = parent_scope_id;
   new_interface.implicit_param_patterns_id = GetLocalParamPatternsId(
       resolver, import_interface.implicit_param_patterns_id);
@@ -2154,7 +2164,7 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
     AddInterfaceDefinition(resolver, import_interface, new_interface,
                            *self_param_id);
   }
-  return ResolveResult::Done(interface_const_id);
+  return ResolveResult::Done(interface_const_id, new_interface.first_decl_id());
 }
 
 static auto TryResolveTypedInst(ImportRefResolver& resolver,
@@ -2747,6 +2757,11 @@ static auto TryResolveInst(ImportRefResolver& resolver, SemIR::InstId inst_id,
                resolver.local_constant_values().GetInstId(result.const_id),
            .generic_id = GetLocalGenericId(resolver, generic_const_id),
            .index = symbolic_const.index});
+      if (result.decl_id.is_valid()) {
+        // Overwrite the abstract symbolic constant given initially to the
+        // declaration with its final concrete symbolic value.
+        resolver.local_constant_values().Set(result.decl_id, result.const_id);
+      }
     }
   } else {
     // Third phase: perform a consistency check and produce the constant we

toolchain/check/testdata/class/generic/import.carbon

@@ -242,10 +242,6 @@ class Class(U:! type) {
 // CHECK:STDOUT: --- foo.impl.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
-// CHECK:STDOUT:   %N: Core.IntLiteral = bind_symbolic_name N, 0 [symbolic]
-// CHECK:STDOUT:   %Int: type = class_type @Int, @Int(%N) [symbolic]
-// CHECK:STDOUT:   %Convert.type.2: type = fn_type @Convert.2, @impl.1(%N) [symbolic]
-// CHECK:STDOUT:   %Convert.2: %Convert.type.2 = struct_value () [symbolic]
 // CHECK:STDOUT:   %int_32: Core.IntLiteral = int_value 32 [template]
 // CHECK:STDOUT:   %i32: type = class_type @Int, @Int(%int_32) [template]
 // CHECK:STDOUT:   %T: type = bind_symbolic_name T, 0 [symbolic]
@@ -271,14 +267,16 @@ class Class(U:! type) {
 // CHECK:STDOUT:   %CompleteClass.elem.2: type = unbound_element_type %CompleteClass.2, %i32 [template]
 // CHECK:STDOUT:   %F.type.3: type = fn_type @F.1, @CompleteClass(%i32) [template]
 // CHECK:STDOUT:   %F.3: %F.type.3 = struct_value () [template]
-// CHECK:STDOUT:   %int_1: Core.IntLiteral = int_value 1 [template]
+// CHECK:STDOUT:   %int_1.1: Core.IntLiteral = int_value 1 [template]
 // CHECK:STDOUT:   %struct_type.n.2: type = struct_type {.n: Core.IntLiteral} [template]
 // CHECK:STDOUT:   %Convert.type.9: type = fn_type @Convert.1, @ImplicitAs(%i32) [template]
 // CHECK:STDOUT:   %impl_witness.19: <witness> = impl_witness (imports.%import_ref.14), @impl.1(%int_32) [template]
-// CHECK:STDOUT:   %Convert.bound: <bound method> = bound_method %int_1, %Convert.2 [symbolic]
-// CHECK:STDOUT:   %Convert.specific_fn: <specific function> = specific_function %Convert.bound, @Convert.2(%N) [symbolic]
-// CHECK:STDOUT:   %int.convert_checked: init %Int = call %Convert.specific_fn(%int_1) [symbolic]
-// CHECK:STDOUT:   %CompleteClass.val: %CompleteClass.2 = struct_value (%int.convert_checked) [symbolic]
+// CHECK:STDOUT:   %Convert.type.10: type = fn_type @Convert.2, @impl.1(%int_32) [template]
+// CHECK:STDOUT:   %Convert.10: %Convert.type.10 = struct_value () [template]
+// CHECK:STDOUT:   %Convert.bound: <bound method> = bound_method %int_1.1, %Convert.10 [template]
+// CHECK:STDOUT:   %Convert.specific_fn: <specific function> = specific_function %Convert.bound, @Convert.2(%int_32) [template]
+// CHECK:STDOUT:   %int_1.2: %i32 = int_value 1 [template]
+// CHECK:STDOUT:   %CompleteClass.val: %CompleteClass.2 = struct_value (%int_1.2) [template]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: imports {
@@ -374,17 +372,17 @@ class Class(U:! type) {
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @F.2() -> %return.param_patt: %CompleteClass.2 [from "foo.carbon"] {
 // CHECK:STDOUT: !entry:
-// CHECK:STDOUT:   %int_1: Core.IntLiteral = int_value 1 [template = constants.%int_1]
+// CHECK:STDOUT:   %int_1: Core.IntLiteral = int_value 1 [template = constants.%int_1.1]
 // CHECK:STDOUT:   %.loc9_17.1: %struct_type.n.2 = struct_literal (%int_1)
-// CHECK:STDOUT:   %impl.elem0: %Convert.type.9 = impl_witness_access constants.%impl_witness.19, element0 [symbolic = constants.%Convert.2]
-// CHECK:STDOUT:   %Convert.bound: <bound method> = bound_method %int_1, %impl.elem0 [symbolic = constants.%Convert.bound]
-// CHECK:STDOUT:   %Convert.specific_fn: <specific function> = specific_function %Convert.bound, @Convert.2(constants.%N) [symbolic = constants.%Convert.specific_fn]
-// CHECK:STDOUT:   %int.convert_checked: init %Int = call %Convert.specific_fn(%int_1) [symbolic = constants.%int.convert_checked]
-// CHECK:STDOUT:   %.loc9_17.2: init %i32 = converted %int_1, %int.convert_checked [symbolic = constants.%int.convert_checked]
+// CHECK:STDOUT:   %impl.elem0: %Convert.type.9 = impl_witness_access constants.%impl_witness.19, element0 [template = constants.%Convert.10]
+// CHECK:STDOUT:   %Convert.bound: <bound method> = bound_method %int_1, %impl.elem0 [template = constants.%Convert.bound]
+// CHECK:STDOUT:   %Convert.specific_fn: <specific function> = specific_function %Convert.bound, @Convert.2(constants.%int_32) [template = constants.%Convert.specific_fn]
+// CHECK:STDOUT:   %int.convert_checked: init %i32 = call %Convert.specific_fn(%int_1) [template = constants.%int_1.2]
+// CHECK:STDOUT:   %.loc9_17.2: init %i32 = converted %int_1, %int.convert_checked [template = constants.%int_1.2]
 // CHECK:STDOUT:   %.loc9_17.3: ref %i32 = class_element_access %return, element0
-// CHECK:STDOUT:   %.loc9_17.4: init %i32 = initialize_from %.loc9_17.2 to %.loc9_17.3 [symbolic = constants.%int.convert_checked]
-// CHECK:STDOUT:   %.loc9_17.5: init %CompleteClass.2 = class_init (%.loc9_17.4), %return [symbolic = constants.%CompleteClass.val]
-// CHECK:STDOUT:   %.loc9_18: init %CompleteClass.2 = converted %.loc9_17.1, %.loc9_17.5 [symbolic = constants.%CompleteClass.val]
+// CHECK:STDOUT:   %.loc9_17.4: init %i32 = initialize_from %.loc9_17.2 to %.loc9_17.3 [template = constants.%int_1.2]
+// CHECK:STDOUT:   %.loc9_17.5: init %CompleteClass.2 = class_init (%.loc9_17.4), %return [template = constants.%CompleteClass.val]
+// CHECK:STDOUT:   %.loc9_18: init %CompleteClass.2 = converted %.loc9_17.1, %.loc9_17.5 [template = constants.%CompleteClass.val]
 // CHECK:STDOUT:   return %.loc9_18 to %return
 // CHECK:STDOUT: }
 // CHECK:STDOUT: