Ensure an imported `Class`'s `NameScope` is allocated in phase 2 (#5548)

Entities, such as `Function`s may be created during phase 2 and need to
read the `Class`'s `scope_id` at that point, so it must be made
available earlier (in phase 2, rather than 3) when importing.

(thanks @zygoloid for explaining this all to me)

I'll look into other instances of this 3 phase lookup to see if they
have
similar bugs/if I can create test cases to tickle them as follow-ups.

---------

Co-authored-by: Richard Smith <richard@metafoo.co.uk>

toolchain/check/import_ref.cpp

@@ -1564,6 +1564,12 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver, SemIR::Call inst)
        .args_id = GetLocalCanonicalInstBlockId(resolver, inst.args_id, args)});
 }
 
+static auto AddPlaceholderNameScope(ImportContext& context)
+    -> SemIR::NameScopeId {
+  return context.local_name_scopes().Add(
+      SemIR::InstId::None, SemIR::NameId::None, SemIR::NameScopeId::None);
+}
+
 // Makes an incomplete class. This is necessary even with classes with a
 // complete declaration, because things such as `Self` may refer back to the
 // type.
@@ -1582,7 +1588,10 @@ static auto MakeIncompleteClass(ImportContext& context,
       {GetIncompleteLocalEntityBase(context, class_decl_id, import_class),
        {.self_type_id = SemIR::TypeId::None,
         .inheritance_kind = import_class.inheritance_kind,
-        .is_dynamic = import_class.is_dynamic}});
+        .is_dynamic = import_class.is_dynamic,
+        .scope_id = import_class.is_complete()
+                        ? AddPlaceholderNameScope(context)
+                        : SemIR::NameScopeId::None}});
 
   if (import_class.has_parameters()) {
     class_decl.type_id = GetGenericClassType(
@@ -1606,15 +1615,14 @@ static auto AddClassDefinition(ImportContext& context,
 
   new_class.complete_type_witness_id = complete_type_witness_id;
 
-  new_class.scope_id = context.local_name_scopes().Add(
-      new_class.first_owning_decl_id, SemIR::NameId::None,
-      new_class.parent_scope_id);
   auto& new_scope = context.local_name_scopes().Get(new_class.scope_id);
   const auto& import_scope =
       context.import_name_scopes().Get(import_class.scope_id);
 
   // Push a block so that we can add scoped instructions to it.
   context.local_context().inst_block_stack().Push();
+  new_scope.Set(new_class.first_owning_decl_id, SemIR::NameId::None,
+                new_class.parent_scope_id);
   AddNameScopeImportRefs(context, import_scope, new_scope);
   new_class.body_block_id = context.local_context().inst_block_stack().Pop();
 

toolchain/lower/testdata/class/import.carbon

@@ -0,0 +1,62 @@
+// 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
+//
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/lower/testdata/class/import.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/lower/testdata/class/import.carbon
+
+// --- a.carbon
+
+library "X";
+base class D[T:! type](X:! T) {
+}
+class C {
+  fn F();
+  extend base: D(F);
+}
+
+// --- b.carbon
+
+import library "X";
+fn Run() {
+  C.F();
+}
+
+// CHECK:STDOUT: ; ModuleID = 'a.carbon'
+// CHECK:STDOUT: source_filename = "a.carbon"
+// CHECK:STDOUT:
+// CHECK:STDOUT: declare void @_CF.C.Main()
+// CHECK:STDOUT:
+// CHECK:STDOUT: !llvm.module.flags = !{!0, !1}
+// CHECK:STDOUT: !llvm.dbg.cu = !{!2}
+// CHECK:STDOUT:
+// CHECK:STDOUT: !0 = !{i32 7, !"Dwarf Version", i32 5}
+// CHECK:STDOUT: !1 = !{i32 2, !"Debug Info Version", i32 3}
+// CHECK:STDOUT: !2 = distinct !DICompileUnit(language: DW_LANG_C, file: !3, producer: "carbon", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug)
+// CHECK:STDOUT: !3 = !DIFile(filename: "a.carbon", directory: "")
+// CHECK:STDOUT: ; ModuleID = 'b.carbon'
+// CHECK:STDOUT: source_filename = "b.carbon"
+// CHECK:STDOUT:
+// CHECK:STDOUT: define void @main() !dbg !4 {
+// CHECK:STDOUT: entry:
+// CHECK:STDOUT:   call void @_CF.C.Main(), !dbg !7
+// CHECK:STDOUT:   ret void, !dbg !8
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: declare void @_CF.C.Main()
+// CHECK:STDOUT:
+// CHECK:STDOUT: !llvm.module.flags = !{!0, !1}
+// CHECK:STDOUT: !llvm.dbg.cu = !{!2}
+// CHECK:STDOUT:
+// CHECK:STDOUT: !0 = !{i32 7, !"Dwarf Version", i32 5}
+// CHECK:STDOUT: !1 = !{i32 2, !"Debug Info Version", i32 3}
+// CHECK:STDOUT: !2 = distinct !DICompileUnit(language: DW_LANG_C, file: !3, producer: "carbon", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug)
+// CHECK:STDOUT: !3 = !DIFile(filename: "b.carbon", directory: "")
+// CHECK:STDOUT: !4 = distinct !DISubprogram(name: "Run", linkageName: "main", scope: null, file: !3, line: 3, type: !5, spFlags: DISPFlagDefinition, unit: !2)
+// CHECK:STDOUT: !5 = !DISubroutineType(types: !6)
+// CHECK:STDOUT: !6 = !{}
+// CHECK:STDOUT: !7 = !DILocation(line: 4, column: 3, scope: !4)
+// CHECK:STDOUT: !8 = !DILocation(line: 3, column: 1, scope: !4)

toolchain/sem_ir/name_scope.h

@@ -195,6 +195,12 @@ class NameScope : public Printable<NameScope> {
     extended_scopes_.push_back(extended_scope);
   }
 
+  auto Set(InstId inst_id, NameId name_id, NameScopeId parent_scope_id) {
+    inst_id_ = inst_id;
+    name_id_ = name_id;
+    parent_scope_id_ = parent_scope_id;
+  }
+
   auto inst_id() const -> InstId { return inst_id_; }
 
   auto name_id() const -> NameId { return name_id_; }