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.

Author: jonmeow

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,15 +403,15 @@ 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;
       }
 
       // Diagnose explicit imports from `Main`.
       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;

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:

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)

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,

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: