Explorar o código

Refactor handling of cross-package imports. (#3783)

This revamps the support for cross-package imports, making them look
more like a namespace. The planned model is mentioned on
[#toolchain](https://discord.com/channels/655572317891461132/655578254970716160/1217586076022210670).
This does not implement name lookup into the new namespace structure.

A few key changes in this PR (it's a little sprawling) are:

- Moves logic for adding package imports from context.* to import.*
- Remove SemIR::Import, which was the prior model. This is instead now a
SemIR::Namespace with the NameScope getting a new import_ir_scopes
field.
- Allow SemIR::Namespace to use Parse::ImportDirectiveId in addition to
the prior Parse::NamespaceId
- The import_ir_scopes field includes a NameScopeId so that as we
traverse to child namespaces, we can directly perform name lookup in the
other IR.
- is_closed_import now tracks whether a namespace comes from a different
package. This has a diagnostic implemented in decl_name_stack.
Jon Ross-Perkins %!s(int64=2) %!d(string=hai) anos
pai
achega
8567e02aa7

+ 29 - 25
toolchain/check/check.cpp

@@ -114,12 +114,13 @@ struct UnitInfo {
   struct PackageImports {
     // Use the constructor so that the SmallVector is only constructed
     // as-needed.
-    explicit PackageImports(Parse::NodeId node) : node(node) {}
+    explicit PackageImports(Parse::ImportDirectiveId node_id)
+        : node_id(node_id) {}
 
     // The first `import` directive in the file, which declared the package's
     // identifier (even if the import failed). Used for associating diagnostics
     // not specific to a single import.
-    Parse::NodeId node;
+    Parse::ImportDirectiveId node_id;
     // Whether there's an import that failed to load.
     bool has_load_error = false;
     // The list of valid imports.
@@ -181,10 +182,11 @@ static auto InitPackageScopeAndImports(Context& context, UnitInfo& unit_info)
     bool error_in_import = self_import->second.has_load_error;
     for (const auto& import : self_import->second.imports) {
       const auto& import_sem_ir = **import.unit_info->unit->sem_ir;
-      Import(context, namespace_type_id, import_sem_ir);
-      error_in_import = error_in_import || import_sem_ir.name_scopes()
-                                               .Get(SemIR::NameScopeId::Package)
-                                               .has_error;
+      ImportLibraryFromCurrentPackage(context, namespace_type_id,
+                                      import_sem_ir);
+      error_in_import |= import_sem_ir.name_scopes()
+                             .Get(SemIR::NameScopeId::Package)
+                             .has_error;
     }
 
     // If an import of the current package caused an error for the imported
@@ -210,8 +212,9 @@ static auto InitPackageScopeAndImports(Context& context, UnitInfo& unit_info)
     for (auto import : package_imports.imports) {
       sem_irs.push_back(&**import.unit_info->unit->sem_ir);
     }
-    context.AddPackageImports(package_imports.node, package_id, sem_irs,
-                              package_imports.has_load_error);
+    ImportLibrariesFromOtherPackage(context, namespace_type_id,
+                                    package_imports.node_id, package_id,
+                                    sem_irs, package_imports.has_load_error);
   }
 
   context.import_ir_constant_values().resize(
@@ -338,12 +341,12 @@ static auto TrackImport(
   if (explicit_import_map) {
     // Diagnose redundant imports.
     if (auto [insert_it, success] =
-            explicit_import_map->insert({import_key, import.node});
+            explicit_import_map->insert({import_key, import.node_id});
         !success) {
       CARBON_DIAGNOSTIC(RepeatedImport, Error,
                         "Library imported more than once.");
       CARBON_DIAGNOSTIC(FirstImported, Note, "First import here.");
-      unit_info.emitter.Build(import.node, RepeatedImport)
+      unit_info.emitter.Build(import.node_id, RepeatedImport)
           .Note(insert_it->second, FirstImported)
           .Emit();
       return;
@@ -377,7 +380,7 @@ static auto TrackImport(
       CARBON_DIAGNOSTIC(ImportSelf, Error, "File cannot import itself.");
       bool is_impl =
           !packaging || packaging->api_or_impl == Parse::Tree::ApiOrImpl::Impl;
-      unit_info.emitter.Emit(import.node,
+      unit_info.emitter.Emit(import.node_id,
                              is_impl ? ExplicitImportApi : ImportSelf);
       return;
     }
@@ -388,7 +391,7 @@ static auto TrackImport(
         is_import_default_library) {
       CARBON_DIAGNOSTIC(ImportMainDefaultLibrary, Error,
                         "Cannot import `Main//default`.");
-      unit_info.emitter.Emit(import.node, ImportMainDefaultLibrary);
+      unit_info.emitter.Emit(import.node_id, ImportMainDefaultLibrary);
 
       return;
     }
@@ -400,7 +403,7 @@ static auto TrackImport(
         CARBON_DIAGNOSTIC(
             ImportCurrentPackageByName, Error,
             "Imports from the current package must omit the package name.");
-        unit_info.emitter.Emit(import.node, ImportCurrentPackageByName);
+        unit_info.emitter.Emit(import.node_id, ImportCurrentPackageByName);
         return;
       }
 
@@ -408,7 +411,7 @@ static auto TrackImport(
       if (is_explicit_main) {
         CARBON_DIAGNOSTIC(ImportMainPackage, Error,
                           "Cannot import `Main` from other packages.");
-        unit_info.emitter.Emit(import.node, ImportMainPackage);
+        unit_info.emitter.Emit(import.node_id, ImportMainPackage);
         return;
       }
     }
@@ -420,9 +423,9 @@ static auto TrackImport(
   }
 
   // Get the package imports.
-  auto package_imports_it =
-      unit_info.package_imports_map.try_emplace(import.package_id, import.node)
-          .first;
+  auto package_imports_it = unit_info.package_imports_map
+                                .try_emplace(import.package_id, import.node_id)
+                                .first;
 
   if (auto api = api_map.find(import_key); api != api_map.end()) {
     // Add references between the file and imported api.
@@ -435,8 +438,9 @@ static auto TrackImport(
     CARBON_DIAGNOSTIC(LibraryApiNotFound, Error,
                       "Corresponding API not found.");
     CARBON_DIAGNOSTIC(ImportNotFound, Error, "Imported API not found.");
-    unit_info.emitter.Emit(
-        import.node, explicit_import_map ? ImportNotFound : LibraryApiNotFound);
+    unit_info.emitter.Emit(import.node_id, explicit_import_map
+                                               ? ImportNotFound
+                                               : LibraryApiNotFound);
   }
 }
 
@@ -463,9 +467,9 @@ static auto BuildApiMapAndDiagnosePackaging(
                         "`Main//default` must omit `package` directive.");
       CARBON_DIAGNOSTIC(ExplicitMainLibrary, Error,
                         "Use `library` directive in `Main` package libraries.");
-      unit_info.emitter.Emit(packaging->names.node, import_key.second.empty()
-                                                        ? ExplicitMainPackage
-                                                        : ExplicitMainLibrary);
+      unit_info.emitter.Emit(packaging->names.node_id,
+                             import_key.second.empty() ? ExplicitMainPackage
+                                                       : ExplicitMainLibrary);
       continue;
     }
 
@@ -485,7 +489,7 @@ static auto BuildApiMapAndDiagnosePackaging(
           CARBON_DIAGNOSTIC(DuplicateLibraryApi, Error,
                             "Library's API previously provided by `{0}`.",
                             std::string);
-          unit_info.emitter.Emit(packaging->names.node, DuplicateLibraryApi,
+          unit_info.emitter.Emit(packaging->names.node_id, DuplicateLibraryApi,
                                  prev_filename.str());
         } else {
           CARBON_DIAGNOSTIC(DuplicateMainApi, Error,
@@ -512,7 +516,7 @@ static auto BuildApiMapAndDiagnosePackaging(
                           "File extension of `{0}` required for `{1}`.",
                           llvm::StringLiteral, Lex::TokenKind);
         auto diag = unit_info.emitter.Build(
-            packaging ? packaging->names.node : Parse::NodeId::Invalid,
+            packaging ? packaging->names.node_id : Parse::NodeId::Invalid,
             IncorrectExtension, want_ext,
             is_impl ? Lex::TokenKind::Impl : Lex::TokenKind::Api);
         if (is_api_with_impl_ext) {
@@ -608,7 +612,7 @@ auto CheckParseTrees(const SemIR::File& builtin_ir,
               CARBON_DIAGNOSTIC(ImportCycleDetected, Error,
                                 "Import cannot be used due to a cycle. Cycle "
                                 "must be fixed to import.");
-              unit_info.emitter.Emit(import_it->names.node,
+              unit_info.emitter.Emit(import_it->names.node_id,
                                      ImportCycleDetected);
               // Make this look the same as an import which wasn't found.
               package_imports.has_load_error = true;

+ 4 - 42
toolchain/check/context.cpp

@@ -133,14 +133,14 @@ auto Context::ReplaceInstBeforeConstantUse(
   constant_values().Set(inst_id, const_id);
 }
 
-auto Context::DiagnoseDuplicateName(SemIR::InstId dup_def_id,
-                                    SemIR::InstId prev_def_id) -> void {
+auto Context::DiagnoseDuplicateName(SemIRLocation dup_def,
+                                    SemIRLocation prev_def) -> void {
   CARBON_DIAGNOSTIC(NameDeclDuplicate, Error,
                     "Duplicate name being declared in the same scope.");
   CARBON_DIAGNOSTIC(NameDeclPrevious, Note,
                     "Name is previously declared here.");
-  emitter_->Build(dup_def_id, NameDeclDuplicate)
-      .Note(prev_def_id, NameDeclPrevious)
+  emitter_->Build(dup_def, NameDeclDuplicate)
+      .Note(prev_def, NameDeclPrevious)
       .Emit();
 }
 
@@ -182,43 +182,6 @@ auto Context::NoteUndefinedInterface(SemIR::InterfaceId interface_id,
   }
 }
 
-auto Context::AddPackageImports(Parse::NodeId import_node,
-                                IdentifierId package_id,
-                                llvm::ArrayRef<const SemIR::File*> sem_irs,
-                                bool has_load_error) -> void {
-  CARBON_CHECK(has_load_error || !sem_irs.empty())
-      << "There should be either a load error or at least one IR.";
-
-  auto name_id = SemIR::NameId::ForIdentifier(package_id);
-
-  SemIR::ImportIRId first_id(import_irs().size());
-  for (const auto* sem_ir : sem_irs) {
-    import_irs().Add(sem_ir);
-  }
-  if (has_load_error) {
-    import_irs().Add(nullptr);
-  }
-  SemIR::ImportIRId last_id(import_irs().size() - 1);
-
-  auto type_id = GetBuiltinType(SemIR::BuiltinKind::NamespaceType);
-  auto inst_id =
-      AddInst({import_node, SemIR::Import{.type_id = type_id,
-                                          .first_import_ir_id = first_id,
-                                          .last_import_ir_id = last_id}});
-
-  // Add the import to lookup. Should always succeed because imports will be
-  // uniquely named.
-  AddNameToLookup(name_id, inst_id);
-  // Add a name for formatted output. This isn't used in name lookup in order
-  // to reduce indirection, but it's separate from the Import because it
-  // otherwise fits in an Inst.
-  auto bind_name_id = bind_names().Add(
-      {.name_id = name_id, .enclosing_scope_id = SemIR::NameScopeId::Package});
-  AddInst({import_node, SemIR::BindName{.type_id = type_id,
-                                        .bind_name_id = bind_name_id,
-                                        .value_id = inst_id}});
-}
-
 auto Context::AddNameToLookup(SemIR::NameId name_id, SemIR::InstId target_id)
     -> void {
   if (auto existing = scope_stack().LookupOrAddName(name_id, target_id);
@@ -837,7 +800,6 @@ class TypeCompleter {
       case SemIR::FieldDecl::Kind:
       case SemIR::FunctionDecl::Kind:
       case SemIR::ImplDecl::Kind:
-      case SemIR::Import::Kind:
       case SemIR::ImportRefUnused::Kind:
       case SemIR::InitializeFrom::Kind:
       case SemIR::InterfaceDecl::Kind:

+ 2 - 8
toolchain/check/context.h

@@ -106,12 +106,6 @@ class Context {
     sem_ir().insts().SetNodeId(inst_id, node_id);
   }
 
-  // Adds a package's imports to name lookup, with all libraries together.
-  // sem_irs will all be non-null; has_load_error must be used for any errors.
-  auto AddPackageImports(Parse::NodeId import_node, IdentifierId package_id,
-                         llvm::ArrayRef<const SemIR::File*> sem_irs,
-                         bool has_load_error) -> void;
-
   // Adds a name to name lookup. Prints a diagnostic for name conflicts.
   auto AddNameToLookup(SemIR::NameId name_id, SemIR::InstId target_id) -> void;
 
@@ -138,8 +132,8 @@ class Context {
       -> SemIR::InstId;
 
   // Prints a diagnostic for a duplicate name.
-  auto DiagnoseDuplicateName(SemIR::InstId dup_def_id,
-                             SemIR::InstId prev_def_id) -> void;
+  auto DiagnoseDuplicateName(SemIRLocation dup_def, SemIRLocation prev_def)
+      -> void;
 
   // Prints a diagnostic for a missing name.
   auto DiagnoseNameNotFound(Parse::NodeId node_id, SemIR::NameId name_id)

+ 13 - 0
toolchain/check/decl_name_stack.cpp

@@ -214,6 +214,19 @@ auto DeclNameStack::UpdateScopeIfNeeded(NameContext& name_context,
       auto scope_id = resolved_inst.As<SemIR::Namespace>().name_scope_id;
       name_context.state = NameContext::State::Resolved;
       name_context.target_scope_id = scope_id;
+      auto& scope = context_->name_scopes().Get(scope_id);
+      if (scope.is_closed_import) {
+        CARBON_DIAGNOSTIC(QualifiedDeclOutsidePackage, Error,
+                          "Imported packages cannot be used for declarations.");
+        CARBON_DIAGNOSTIC(QualifiedDeclOutsidePackageSource, Note,
+                          "Package imported here.");
+        context_->emitter()
+            .Build(name_context.node_id, QualifiedDeclOutsidePackage)
+            .Note(scope.inst_id, QualifiedDeclOutsidePackageSource)
+            .Emit();
+        // Only error once per package.
+        scope.is_closed_import = false;
+      }
       if (!is_unqualified) {
         PushNameQualifierScope(*context_, name_context.resolved_inst_id,
                                scope_id,

+ 0 - 1
toolchain/check/eval.cpp

@@ -506,7 +506,6 @@ auto TryEvalInst(Context& context, SemIR::InstId inst_id, SemIR::Inst inst)
     case SemIR::BranchIf::Kind:
     case SemIR::BranchWithArg::Kind:
     case SemIR::ImplDecl::Kind:
-    case SemIR::Import::Kind:
     case SemIR::Param::Kind:
     case SemIR::ReturnExpr::Kind:
     case SemIR::Return::Kind:

+ 84 - 36
toolchain/check/import.cpp

@@ -70,6 +70,50 @@ static auto CopyNameFromImportIR(Context& context,
   return import_name_id;
 }
 
+// Adds a namespace to the IR. The bool on return is true if there was a name
+// conflict. diagnose_duplicate_namespace is used when handling a cross-package
+// import, where an existing namespace is in the current package and the new
+// namespace is a different package.
+static auto AddNamespace(
+    Context& context, SemIR::TypeId namespace_type_id,
+    Parse::ImportDirectiveId node_id, SemIR::NameId name_id,
+    SemIR::NameScopeId enclosing_scope_id, bool diagnose_duplicate_namespace,
+    std::optional<llvm::function_ref<SemIR::InstId()>> make_import_id)
+    -> std::pair<SemIR::NameScopeId, bool> {
+  auto& enclosing_scope = context.name_scopes().Get(enclosing_scope_id);
+  auto [it, success] =
+      enclosing_scope.names.insert({name_id, SemIR::InstId::Invalid});
+  if (!success) {
+    if (auto namespace_inst =
+            context.insts().TryGetAs<SemIR::Namespace>(it->second)) {
+      if (diagnose_duplicate_namespace) {
+        context.DiagnoseDuplicateName(node_id, it->second);
+      }
+      return {namespace_inst->name_scope_id, true};
+    }
+  }
+
+  auto import_id =
+      make_import_id ? (*make_import_id)() : SemIR::InstId::Invalid;
+  auto namespace_inst = SemIR::Namespace{
+      namespace_type_id, SemIR::NameScopeId::Invalid, import_id};
+  // Use the invalid node because there's no node to associate with.
+  auto namespace_id = context.AddPlaceholderInst({node_id, namespace_inst});
+  namespace_inst.name_scope_id =
+      context.name_scopes().Add(namespace_id, name_id, enclosing_scope_id);
+  context.ReplaceInstBeforeConstantUse(namespace_id, {node_id, namespace_inst});
+
+  // Diagnose if there's a name conflict, but still produce the namespace to
+  // supersede the name conflict in order to avoid repeat diagnostics.
+  if (!success) {
+    context.DiagnoseDuplicateName(namespace_id, it->second);
+  }
+
+  it->second = namespace_id;
+  return {namespace_inst.name_scope_id, false};
+}
+
+// Adds a copied namespace to the cache.
 static auto CacheCopiedNamespace(
     llvm::DenseMap<SemIR::NameScopeId, SemIR::NameScopeId>& copied_namespaces,
     SemIR::NameScopeId import_scope_id, SemIR::NameScopeId to_scope_id)
@@ -90,42 +134,19 @@ static auto CopySingleNameScopeFromImportIR(
     SemIR::NameScopeId import_scope_id, SemIR::NameScopeId enclosing_scope_id,
     SemIR::NameId name_id, SemIR::TypeId namespace_type_id)
     -> SemIR::NameScopeId {
-  auto& scope = context.name_scopes().Get(enclosing_scope_id);
-  auto [it, success] = scope.names.insert({name_id, SemIR::InstId::Invalid});
-  if (!success) {
-    if (auto namespace_inst =
-            context.insts().TryGetAs<SemIR::Namespace>(it->second)) {
-      // Namespaces are open, so we can append to the existing one even if it
-      // comes from a different file.
-      CacheCopiedNamespace(copied_namespaces, import_scope_id,
-                           namespace_inst->name_scope_id);
-      return namespace_inst->name_scope_id;
-    }
-  }
-
   // Produce the namespace for the entry.
-  auto ref_id = context.AddInst(SemIR::ImportRefUsed{
-      .type_id = namespace_type_id, .ir_id = ir_id, .inst_id = import_inst_id});
-  auto namespace_inst =
-      SemIR::Namespace{namespace_type_id, SemIR::NameScopeId::Invalid, ref_id};
-  // Use the invalid node because there's no node to associate with.
-  auto namespace_id =
-      context.AddPlaceholderInst({Parse::NodeId::Invalid, namespace_inst});
-  namespace_inst.name_scope_id =
-      context.name_scopes().Add(namespace_id, name_id, enclosing_scope_id);
-  context.ReplaceInstBeforeConstantUse(
-      namespace_id, {Parse::NodeId::Invalid, namespace_inst});
-
-  // Diagnose if there's a name conflict, but still produce the namespace to
-  // supersede the name conflict in order to avoid repeat diagnostics.
-  if (!success) {
-    context.DiagnoseDuplicateName(namespace_id, it->second);
-  }
+  auto make_import_id = [&]() {
+    return context.AddInst(SemIR::ImportRefUsed{.type_id = namespace_type_id,
+                                                .ir_id = ir_id,
+                                                .inst_id = import_inst_id});
+  };
+  auto [namespace_scope_id, _] =
+      AddNamespace(context, namespace_type_id, Parse::NodeId::Invalid, name_id,
+                   enclosing_scope_id, /*diagnose_duplicate_namespace=*/false,
+                   make_import_id);
 
-  it->second = namespace_id;
-  CacheCopiedNamespace(copied_namespaces, import_scope_id,
-                       namespace_inst.name_scope_id);
-  return namespace_inst.name_scope_id;
+  CacheCopiedNamespace(copied_namespaces, import_scope_id, namespace_scope_id);
+  return namespace_scope_id;
 }
 
 // Copies enclosing name scopes from the import IR. Handles the parent
@@ -180,8 +201,9 @@ static auto CopyEnclosingNameScopesFromImportIR(
   return scope_cursor;
 }
 
-auto Import(Context& context, SemIR::TypeId namespace_type_id,
-            const SemIR::File& import_sem_ir) -> void {
+auto ImportLibraryFromCurrentPackage(Context& context,
+                                     SemIR::TypeId namespace_type_id,
+                                     const SemIR::File& import_sem_ir) -> void {
   auto ir_id = context.import_irs().Add(&import_sem_ir);
 
   for (const auto import_inst_id :
@@ -227,4 +249,30 @@ auto Import(Context& context, SemIR::TypeId namespace_type_id,
   }
 }
 
+auto ImportLibrariesFromOtherPackage(Context& context,
+                                     SemIR::TypeId namespace_type_id,
+                                     Parse::ImportDirectiveId node_id,
+                                     IdentifierId package_id,
+                                     llvm::ArrayRef<const SemIR::File*> sem_irs,
+                                     bool has_load_error) -> void {
+  CARBON_CHECK(has_load_error || !sem_irs.empty())
+      << "There should be either a load error or at least one IR.";
+
+  auto name_id = SemIR::NameId::ForIdentifier(package_id);
+
+  auto [namespace_scope_id, is_duplicate] = AddNamespace(
+      context, namespace_type_id, node_id, name_id, SemIR::NameScopeId::Package,
+      /*diagnose_duplicate_namespace=*/true, /*make_import_id=*/std::nullopt);
+
+  auto& scope = context.name_scopes().Get(namespace_scope_id);
+  scope.is_closed_import = !is_duplicate;
+  for (const auto* sem_ir : sem_irs) {
+    scope.import_ir_scopes.push_back(
+        {context.import_irs().Add(sem_ir), SemIR::NameScopeId::Package});
+  }
+  if (has_load_error) {
+    scope.has_error = has_load_error;
+  }
+}
+
 }  // namespace Carbon::Check

+ 20 - 3
toolchain/check/import.h

@@ -10,9 +10,26 @@
 
 namespace Carbon::Check {
 
-// Add imports to the root block.
-auto Import(Context& context, SemIR::TypeId namespace_type_id,
-            const SemIR::File& import_sem_ir) -> void;
+// Add imports from a single library in the current package. This pulls in all
+// names; conflicts for things such as `package.a.b.c` will be flagged even
+// though they are several layers deep.
+auto ImportLibraryFromCurrentPackage(Context& context,
+                                     SemIR::TypeId namespace_type_id,
+                                     const SemIR::File& import_sem_ir) -> void;
+
+// Adds another package's imports to name lookup, with all libraries together.
+// This only adds the package name to lookup, so that `package.ImportedPackage`
+// will resolve, and will provide a name scope that can be used for further
+// qualified name lookups.
+//
+// sem_irs will all be non-null, and may be empty. has_load_error is used to
+// indicate if any library in the package failed to import correctly.
+auto ImportLibrariesFromOtherPackage(Context& context,
+                                     SemIR::TypeId namespace_type_id,
+                                     Parse::ImportDirectiveId node_id,
+                                     IdentifierId package_id,
+                                     llvm::ArrayRef<const SemIR::File*> sem_irs,
+                                     bool has_load_error) -> void;
 
 }  // namespace Carbon::Check
 

+ 276 - 0
toolchain/check/testdata/packages/cross_package_import.carbon

@@ -0,0 +1,276 @@
+// 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
+
+// ============================================================================
+// Setup files
+// ============================================================================
+
+// --- other_fn.carbon
+
+package Other library "fn" api;
+
+fn F() {}
+
+// --- other_fn_conflict.carbon
+
+package Other library "fn_conflict" api;
+
+fn F() {}
+
+// --- other_fn2.carbon
+
+package Other library "fn2" api;
+
+fn F2() {}
+
+// --- main_other_ns.carbon
+
+library "other_ns" api;
+
+namespace Other;
+
+// ============================================================================
+// Test files
+// ============================================================================
+
+// --- fail_main_use_other.carbon
+// TODO: Once name lookup works, nothing should fail here.
+
+library "use_other" api;
+
+import Other library "fn";
+import Other library "fn2";
+
+fn Run() {
+  // CHECK:STDERR: fail_main_use_other.carbon:[[@LINE+3]]:3: ERROR: Name `F` not found.
+  // CHECK:STDERR:   Other.F();
+  // CHECK:STDERR:   ^~~~~~~
+  Other.F();
+  // CHECK:STDERR: fail_main_use_other.carbon:[[@LINE+3]]:3: ERROR: Name `F2` not found.
+  // CHECK:STDERR:   Other.F2();
+  // CHECK:STDERR:   ^~~~~~~~
+  Other.F2();
+}
+
+// --- main_unused_other_ambiguous.carbon
+
+library "unused_other_ambiguous" api;
+
+import Other library "fn";
+import Other library "fn_conflict";
+
+// --- fail_main_use_other_ambiguous.carbon
+// TODO: Once name lookup works, this should be ambiguous.
+
+library "use_other_ambiguous" api;
+
+import Other library "fn";
+import Other library "fn_conflict";
+
+fn Run() {
+  // CHECK:STDERR: fail_main_use_other_ambiguous.carbon:[[@LINE+3]]:3: ERROR: Name `F` not found.
+  // CHECK:STDERR:   Other.F();
+  // CHECK:STDERR:   ^~~~~~~
+  Other.F();
+}
+
+// --- fail_main_namespace_conflict.carbon
+
+library "namespace_conflict" api;
+
+import library "other_ns";
+// CHECK:STDERR: fail_main_namespace_conflict.carbon:[[@LINE+6]]:1: ERROR: Duplicate name being declared in the same scope.
+// CHECK:STDERR: import Other library "fn";
+// CHECK:STDERR: ^~~~~~
+// CHECK:STDERR: main_other_ns.carbon:4:1: Name is previously declared here.
+// CHECK:STDERR: namespace Other;
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~
+import Other library "fn";
+
+fn Other.F() {}
+
+// --- fail_main_reopen_other.carbon
+
+library "reopen_other" api;
+
+import Other library "fn";
+
+// CHECK:STDERR: fail_main_reopen_other.carbon:[[@LINE+6]]:11: ERROR: Imported packages cannot be used for declarations.
+// CHECK:STDERR: namespace Other;
+// CHECK:STDERR:           ^~~~~
+// CHECK:STDERR: fail_main_reopen_other.carbon:[[@LINE-5]]:1: Package imported here.
+// CHECK:STDERR: import Other library "fn";
+// CHECK:STDERR: ^~~~~~
+namespace Other;
+
+// This is not diagnosed after the diagnostic on `namespace Other;`.
+fn Other.G() {}
+
+// --- fail_main_add_to_other.carbon
+
+library "add_to_other" api;
+
+import Other library "fn";
+
+// CHECK:STDERR: fail_main_add_to_other.carbon:[[@LINE+6]]:4: ERROR: Imported packages cannot be used for declarations.
+// CHECK:STDERR: fn Other.G() {}
+// CHECK:STDERR:    ^~~~~
+// CHECK:STDERR: fail_main_add_to_other.carbon:[[@LINE-5]]:1: Package imported here.
+// CHECK:STDERR: import Other library "fn";
+// CHECK:STDERR: ^~~~~~
+fn Other.G() {}
+
+// CHECK:STDOUT: --- other_fn.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .F = %F
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %F: <function> = fn_decl @F [template] {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- other_fn_conflict.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .F = %F
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %F: <function> = fn_decl @F [template] {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- other_fn2.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .F2 = %F2
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %F2: <function> = fn_decl @F2 [template] {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F2() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- main_other_ns.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Other = %Other
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Other: <namespace> = namespace [template] {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- fail_main_use_other.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Other = %Other
+// CHECK:STDOUT:     .Run = %Run
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Other: <namespace> = namespace [template] {}
+// CHECK:STDOUT:   %Run: <function> = fn_decl @Run [template] {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @Run() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %Other.ref.loc12: <namespace> = name_ref Other, file.%Other [template = file.%Other]
+// CHECK:STDOUT:   %F.ref: <error> = name_ref F, <error> [template = <error>]
+// CHECK:STDOUT:   %Other.ref.loc16: <namespace> = name_ref Other, file.%Other [template = file.%Other]
+// CHECK:STDOUT:   %F2.ref: <error> = name_ref F2, <error> [template = <error>]
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- main_unused_other_ambiguous.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Other = %Other
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Other: <namespace> = namespace [template] {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- fail_main_use_other_ambiguous.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Other = %Other
+// CHECK:STDOUT:     .Run = %Run
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Other: <namespace> = namespace [template] {}
+// CHECK:STDOUT:   %Run: <function> = fn_decl @Run [template] {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @Run() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %Other.ref: <namespace> = name_ref Other, file.%Other [template = file.%Other]
+// CHECK:STDOUT:   %F.ref: <error> = name_ref F, <error> [template = <error>]
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- fail_main_namespace_conflict.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Other = %Other
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %import_ref: <namespace> = import_ref ir1, inst+1, used
+// CHECK:STDOUT:   %Other: <namespace> = namespace %import_ref, [template] {
+// CHECK:STDOUT:     .F = %F
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %F: <function> = fn_decl @F [template] {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- fail_main_reopen_other.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Other = %Other
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Other: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .G = %G
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %.loc12: <namespace> = namespace [template] {}
+// CHECK:STDOUT:   %G: <function> = fn_decl @G [template] {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @G() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- fail_main_add_to_other.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Other = %Other
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Other: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .G = %G
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %G: <function> = fn_decl @G [template] {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @G() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:

+ 4 - 3
toolchain/check/testdata/packages/explicit_imports.carbon

@@ -55,9 +55,10 @@ import library "lib";
 // CHECK:STDOUT: --- different_package.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
-// CHECK:STDOUT:   package: <namespace> = namespace [template] {}
-// CHECK:STDOUT:   %import: <namespace> = import ir1, ir2
-// CHECK:STDOUT:   %Api: <namespace> = bind_name Api, %import
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Api = %Api
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Api: <namespace> = namespace [template] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- main_lib_api.carbon

+ 24 - 12
toolchain/check/testdata/packages/fail_cycle.carbon

@@ -50,25 +50,34 @@ import B;
 // CHECK:STDOUT: --- fail_a.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
-// CHECK:STDOUT:   package: <namespace> = namespace [template] {}
-// CHECK:STDOUT:   %import: <namespace> = import ir1, ir1
-// CHECK:STDOUT:   %B: <namespace> = bind_name B, %import
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .B = %B
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %B: <namespace> = namespace [template] {
+// CHECK:STDOUT:     has_error
+// CHECK:STDOUT:   }
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- fail_b.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
-// CHECK:STDOUT:   package: <namespace> = namespace [template] {}
-// CHECK:STDOUT:   %import: <namespace> = import ir1, ir1
-// CHECK:STDOUT:   %C: <namespace> = bind_name C, %import
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .C = %C
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %C: <namespace> = namespace [template] {
+// CHECK:STDOUT:     has_error
+// CHECK:STDOUT:   }
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- fail_c.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
-// CHECK:STDOUT:   package: <namespace> = namespace [template] {}
-// CHECK:STDOUT:   %import: <namespace> = import ir1, ir1
-// CHECK:STDOUT:   %A: <namespace> = bind_name A, %import
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .A = %A
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %A: <namespace> = namespace [template] {
+// CHECK:STDOUT:     has_error
+// CHECK:STDOUT:   }
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- fail_c.impl.carbon
@@ -82,8 +91,11 @@ import B;
 // CHECK:STDOUT: --- fail_cycle_child.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
-// CHECK:STDOUT:   package: <namespace> = namespace [template] {}
-// CHECK:STDOUT:   %import: <namespace> = import ir1, ir1
-// CHECK:STDOUT:   %B: <namespace> = bind_name B, %import
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .B = %B
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %B: <namespace> = namespace [template] {
+// CHECK:STDOUT:     has_error
+// CHECK:STDOUT:   }
 // CHECK:STDOUT: }
 // CHECK:STDOUT:

+ 6 - 3
toolchain/check/testdata/packages/fail_import_invalid.carbon

@@ -133,8 +133,11 @@ import ImportNotFound;
 // CHECK:STDOUT: --- fail_not_found.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
-// CHECK:STDOUT:   package: <namespace> = namespace [template] {}
-// CHECK:STDOUT:   %import: <namespace> = import ir1, ir1
-// CHECK:STDOUT:   %ImportNotFound: <namespace> = bind_name ImportNotFound, %import
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .ImportNotFound = %ImportNotFound
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %ImportNotFound: <namespace> = namespace [template] {
+// CHECK:STDOUT:     has_error
+// CHECK:STDOUT:   }
 // CHECK:STDOUT: }
 // CHECK:STDOUT:

+ 4 - 3
toolchain/check/testdata/packages/fail_import_repeat.carbon

@@ -79,9 +79,10 @@ import library default;
 // CHECK:STDOUT: --- fail_import.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
-// CHECK:STDOUT:   package: <namespace> = namespace [template] {}
-// CHECK:STDOUT:   %import: <namespace> = import ir2, ir3
-// CHECK:STDOUT:   %Api: <namespace> = bind_name Api, %import
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Api = %Api
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Api: <namespace> = namespace [template] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- fail_default_import.carbon

+ 7 - 3
toolchain/diagnostics/diagnostic_kind.def

@@ -210,6 +210,13 @@ CARBON_DIAGNOSTIC_KIND(MissingImplInMemberAccess)
 CARBON_DIAGNOSTIC_KIND(ExpectedInitializerAfterLet)
 CARBON_DIAGNOSTIC_KIND(ExpectedSymbolicBindingInAssociatedConstant)
 
+// Qualified declaration name checking.
+CARBON_DIAGNOSTIC_KIND(QualifiedDeclOutsidePackage)
+CARBON_DIAGNOSTIC_KIND(QualifiedDeclOutsidePackageSource)
+CARBON_DIAGNOSTIC_KIND(QualifiedDeclOutsideScopeEntity)
+CARBON_DIAGNOSTIC_KIND(QualifiedDeclInIncompleteClassScope)
+CARBON_DIAGNOSTIC_KIND(QualifiedDeclInUndefinedInterfaceScope)
+
 CARBON_DIAGNOSTIC_KIND(AddrOfEphemeralRef)
 CARBON_DIAGNOSTIC_KIND(AddrOfNonRef)
 CARBON_DIAGNOSTIC_KIND(AddrOnNonSelfParam)
@@ -262,9 +269,6 @@ CARBON_DIAGNOSTIC_KIND(ReturnStatementMissingExpr)
 CARBON_DIAGNOSTIC_KIND(ImplicitAsConversionFailure)
 CARBON_DIAGNOSTIC_KIND(ExplicitAsConversionFailure)
 CARBON_DIAGNOSTIC_KIND(TypeExprEvaluationFailure)
-CARBON_DIAGNOSTIC_KIND(QualifiedDeclOutsideScopeEntity)
-CARBON_DIAGNOSTIC_KIND(QualifiedDeclInIncompleteClassScope)
-CARBON_DIAGNOSTIC_KIND(QualifiedDeclInUndefinedInterfaceScope)
 CARBON_DIAGNOSTIC_KIND(QualifiedNameInNonScope)
 CARBON_DIAGNOSTIC_KIND(QualifiedNameNonScopeEntity)
 CARBON_DIAGNOSTIC_KIND(QualifiedExprInIncompleteClassScope)

+ 0 - 5
toolchain/lower/handle.cpp

@@ -217,11 +217,6 @@ auto HandleImplDecl(FunctionContext& /*context*/, SemIR::InstId /*inst_id*/,
   FatalErrorIfEncountered(inst);
 }
 
-auto HandleImport(FunctionContext& /*context*/, SemIR::InstId /*inst_id*/,
-                  SemIR::Import inst) -> void {
-  FatalErrorIfEncountered(inst);
-}
-
 auto HandleImportRefUnused(FunctionContext& /*context*/,
                            SemIR::InstId /*inst_id*/,
                            SemIR::ImportRefUnused inst) -> void {

+ 3 - 1
toolchain/parse/handle_import_and_package.cpp

@@ -5,6 +5,7 @@
 #include "toolchain/base/value_store.h"
 #include "toolchain/lex/tokenized_buffer.h"
 #include "toolchain/parse/context.h"
+#include "toolchain/parse/node_ids.h"
 #include "toolchain/parse/node_kind.h"
 
 namespace Carbon::Parse {
@@ -75,7 +76,8 @@ static auto HandleDirectiveContent(Context& context,
                                    NodeKind directive,
                                    llvm::function_ref<void()> on_parse_error)
     -> void {
-  Tree::PackagingNames names{.node = NodeId(state.subtree_start)};
+  Tree::PackagingNames names{
+      .node_id = ImportDirectiveId(NodeId(state.subtree_start))};
   if (directive != NodeKind::LibraryDirective) {
     if (auto package_name_token =
             context.ConsumeIf(Lex::TokenKind::Identifier)) {

+ 1 - 0
toolchain/parse/node_ids.h

@@ -91,6 +91,7 @@ using AnyFunctionDeclId =
 using AnyImplDeclId = NodeIdOneOf<ImplDeclId, ImplDefinitionStartId>;
 using AnyInterfaceDeclId =
     NodeIdOneOf<InterfaceDeclId, InterfaceDefinitionStartId>;
+using AnyNamespaceId = NodeIdOneOf<NamespaceId, ImportDirectiveId>;
 
 // NodeId with kind that is anything but T::Kind.
 template <typename T>

+ 1 - 1
toolchain/parse/tree.h

@@ -57,7 +57,7 @@ class Tree : public Printable<Tree> {
   // Names in packaging, whether the file's packaging or an import. Links back
   // to the node for diagnostics.
   struct PackagingNames {
-    NodeId node;
+    ImportDirectiveId node_id;
     IdentifierId package_id = IdentifierId::Invalid;
     StringLiteralValueId library_id = StringLiteralValueId::Invalid;
   };

+ 0 - 3
toolchain/sem_ir/file.cpp

@@ -234,7 +234,6 @@ static auto GetTypePrecedence(InstKind kind) -> int {
     case FieldDecl::Kind:
     case FunctionDecl::Kind:
     case ImplDecl::Kind:
-    case Import::Kind:
     case ImportRefUnused::Kind:
     case InitializeFrom::Kind:
     case InterfaceDecl::Kind:
@@ -483,7 +482,6 @@ static auto StringifyTypeExprImpl(const SemIR::File& outer_sem_ir,
       case FieldDecl::Kind:
       case FunctionDecl::Kind:
       case ImplDecl::Kind:
-      case Import::Kind:
       case ImportRefUnused::Kind:
       case InitializeFrom::Kind:
       case InterfaceDecl::Kind:
@@ -549,7 +547,6 @@ auto GetExprCategory(const File& file, InstId inst_id) -> ExprCategory {
       case FieldDecl::Kind:
       case FunctionDecl::Kind:
       case ImplDecl::Kind:
-      case Import::Kind:
       case ImportRefUnused::Kind:
       case Namespace::Kind:
       case Return::Kind:

+ 0 - 4
toolchain/sem_ir/formatter.cpp

@@ -503,10 +503,6 @@ class InstNamer {
           CollectNamesInBlock(scope_id, inst.As<ImplDecl>().decl_block_id);
           break;
         }
-        case Import::Kind: {
-          add_inst_name("import");
-          continue;
-        }
         case ImportRefUnused::Kind:
         case ImportRefUsed::Kind: {
           add_inst_name("import_ref");

+ 0 - 1
toolchain/sem_ir/inst_kind.def

@@ -50,7 +50,6 @@ CARBON_SEM_IR_INST_KIND(FacetTypeAccess)
 CARBON_SEM_IR_INST_KIND(FieldDecl)
 CARBON_SEM_IR_INST_KIND(FunctionDecl)
 CARBON_SEM_IR_INST_KIND(ImplDecl)
-CARBON_SEM_IR_INST_KIND(Import)
 CARBON_SEM_IR_INST_KIND(ImportRefUnused)
 CARBON_SEM_IR_INST_KIND(ImportRefUsed)
 CARBON_SEM_IR_INST_KIND(InitializeFrom)

+ 8 - 0
toolchain/sem_ir/name_scope.h

@@ -74,6 +74,14 @@ struct NameScope : Printable<NameScope> {
   // to be missing names as a result of the error, and no further errors are
   // produced for lookup failures in this scope.
   bool has_error = false;
+
+  // True if this is a closed namespace created by importing a package.
+  bool is_closed_import = false;
+
+  // Imported IR scopes that compose this namespace. This will be empty for
+  // scopes that correspond to the current package.
+  llvm::SmallVector<std::pair<SemIR::ImportIRId, SemIR::NameScopeId>, 0>
+      import_ir_scopes;
 };
 
 // Provides a ValueStore wrapper for an API specific to name scopes.

+ 1 - 15
toolchain/sem_ir/typed_insts.h

@@ -457,19 +457,6 @@ struct ImplDecl {
   InstBlockId decl_block_id;
 };
 
-// An import corresponds to some number of IRs. The range of imported IRs is
-// inclusive of last_import_ir_id, and will always be non-empty. If
-// there was an import error, first_import_ir_id will reference a
-// nullptr IR; there should only ever be one nullptr in the range.
-struct Import {
-  // TODO: Should always be an ImportDirectiveId?
-  static constexpr auto Kind = InstKind::Import.Define<Parse::NodeId>("import");
-
-  TypeId type_id;
-  ImportIRId first_import_ir_id;
-  ImportIRId last_import_ir_id;
-};
-
 // Common representation for all kinds of `ImportRef*` node.
 struct AnyImportRef {
   static constexpr InstKind Kinds[] = {InstKind::ImportRefUnused,
@@ -482,7 +469,6 @@ struct AnyImportRef {
 
 // An imported entity that hasn't yet been referenced. If referenced, it should
 // turn into an ImportRefUsed.
-// TODO: Add ImportRefUsed.
 struct ImportRefUnused {
   // No parse node: any parse node logic must use the referenced IR.
   static constexpr auto Kind =
@@ -582,7 +568,7 @@ struct NameRef {
 
 struct Namespace {
   static constexpr auto Kind =
-      InstKind::Namespace.Define<Parse::NamespaceId>("namespace");
+      InstKind::Namespace.Define<Parse::AnyNamespaceId>("namespace");
 
   TypeId type_id;
   NameScopeId name_scope_id;