Apply `is_closed_import` to imported namespaces (#4312)

Spin off from
https://github.com/carbon-language/carbon-lang/pull/4294#discussion_r1752916451

---------

Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>

toolchain/check/import.cpp

@@ -80,6 +80,14 @@ static auto CopyNameFromImportIR(Context& context,
   return import_name_id;
 }
 
+namespace {
+struct NamespaceResult {
+  SemIR::NameScopeId name_scope_id;
+  SemIR::InstId inst_id;
+  bool is_duplicate_of_namespace_in_current_package;
+};
+}  // namespace
+
 // 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
@@ -89,7 +97,7 @@ static auto AddNamespace(Context& context, SemIR::TypeId namespace_type_id,
                          SemIR::NameScopeId parent_scope_id,
                          bool diagnose_duplicate_namespace,
                          llvm::function_ref<SemIR::InstId()> make_import_id)
-    -> std::tuple<SemIR::NameScopeId, SemIR::InstId, bool> {
+    -> NamespaceResult {
   auto* parent_scope = &context.name_scopes().Get(parent_scope_id);
   auto insert_result =
       parent_scope->name_map.Insert(name_id, parent_scope->names.size());
@@ -159,13 +167,14 @@ static auto CacheCopiedNamespace(
 
 // Copies a namespace from the import IR, returning its ID. This may diagnose
 // name conflicts, but that won't change the result because namespaces supersede
-// other names in conflicts. copied_namespaces is optional.
+// other names in conflicts. The bool on return is true if there was a name
+// conflict. copied_namespaces is optional.
 static auto CopySingleNameScopeFromImportIR(
     Context& context, SemIR::TypeId namespace_type_id,
     Map<SemIR::NameScopeId, SemIR::NameScopeId>* copied_namespaces,
     SemIR::ImportIRId ir_id, SemIR::InstId import_inst_id,
     SemIR::NameScopeId import_scope_id, SemIR::NameScopeId parent_scope_id,
-    SemIR::NameId name_id) -> std::pair<SemIR::NameScopeId, SemIR::InstId> {
+    SemIR::NameId name_id) -> NamespaceResult {
   // Produce the namespace for the entry.
   auto make_import_id = [&]() {
     auto entity_name_id = context.entity_names().Add(
@@ -182,19 +191,19 @@ static auto CopySingleNameScopeFromImportIR(
     context.import_ref_ids().push_back(inst_id);
     return inst_id;
   };
-  auto [namespace_scope_id, namespace_inst_id, _] =
+  NamespaceResult result =
       AddNamespace(context, namespace_type_id, name_id, parent_scope_id,
                    /*diagnose_duplicate_namespace=*/false, make_import_id);
 
-  auto namespace_const_id = context.constant_values().Get(namespace_inst_id);
+  auto namespace_const_id = context.constant_values().Get(result.inst_id);
   context.import_ir_constant_values()[ir_id.index].Set(import_inst_id,
                                                        namespace_const_id);
 
   if (copied_namespaces) {
     CacheCopiedNamespace(*copied_namespaces, import_scope_id,
-                         namespace_scope_id);
+                         result.name_scope_id);
   }
-  return {namespace_scope_id, namespace_inst_id};
+  return result;
 }
 
 // Copies ancestor name scopes from the import IR. Handles the parent traversal.
@@ -242,7 +251,7 @@ static auto CopyAncestorNameScopesFromImportIR(
         CopySingleNameScopeFromImportIR(
             context, namespace_type_id, &copied_namespaces, ir_id,
             import_scope.inst_id, import_scope_id, scope_cursor, name_id)
-            .first;
+            .name_scope_id;
   }
 
   return scope_cursor;
@@ -372,7 +381,7 @@ auto ImportApiFile(Context& context, SemIR::TypeId namespace_type_id,
             SemIR::ImportIRId::ApiForImpl, todo_scope.api_inst_id,
             todo_scope.api_scope_id, todo_scope.impl_parent_scope_id,
             todo_scope.impl_name_id)
-            .first;
+            .name_scope_id;
     ImportScopeFromApiFile(context, api_sem_ir, todo_scope.api_scope_id,
                            impl_scope_id, todo_scopes);
   }
@@ -436,13 +445,13 @@ auto ImportLibrariesFromOtherPackage(Context& context,
 
   auto name_id = SemIR::NameId::ForIdentifier(package_id);
 
-  auto [namespace_scope_id, namespace_inst_id, is_duplicate] = AddNamespace(
+  NamespaceResult result = AddNamespace(
       context, namespace_type_id, name_id, SemIR::NameScopeId::Package,
       /*diagnose_duplicate_namespace=*/true, [&] { return import_decl_id; });
-  auto namespace_const_id = context.constant_values().Get(namespace_inst_id);
+  auto namespace_const_id = context.constant_values().Get(result.inst_id);
 
-  auto& scope = context.name_scopes().Get(namespace_scope_id);
-  scope.is_closed_import = !is_duplicate;
+  auto& scope = context.name_scopes().Get(result.name_scope_id);
+  scope.is_closed_import = !result.is_duplicate_of_namespace_in_current_package;
   for (auto import_ir : import_irs) {
     auto ir_id = AddImportIR(context, import_ir);
     scope.import_ir_scopes.push_back({ir_id, SemIR::NameScopeId::Package});
@@ -500,13 +509,13 @@ static auto AddNamespaceFromOtherPackage(Context& context,
     -> SemIR::InstId {
   auto namespace_type_id =
       context.GetBuiltinType(SemIR::BuiltinInstKind::NamespaceType);
-  auto [new_scope_id, namespace_inst_id] = CopySingleNameScopeFromImportIR(
+  NamespaceResult result = CopySingleNameScopeFromImportIR(
       context, namespace_type_id, /*copied_namespaces=*/nullptr, import_ir_id,
       import_inst_id, import_ns.name_scope_id, parent_scope_id, name_id);
-  context.name_scopes()
-      .Get(new_scope_id)
-      .import_ir_scopes.push_back({import_ir_id, import_ns.name_scope_id});
-  return namespace_inst_id;
+  auto& scope = context.name_scopes().Get(result.name_scope_id);
+  scope.is_closed_import = !result.is_duplicate_of_namespace_in_current_package;
+  scope.import_ir_scopes.push_back({import_ir_id, import_ns.name_scope_id});
+  return result.inst_id;
 }
 
 auto ImportNameFromOtherPackage(

toolchain/check/testdata/namespace/fail_conflict_imported_namespace_nested.carbon

@@ -0,0 +1,107 @@
+// 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/check/testdata/namespace/fail_conflict_imported_namespace_nested.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/namespace/fail_conflict_imported_namespace_nested.carbon
+
+// --- fn.carbon
+
+package Other;
+namespace Nested;
+
+// --- fail_conflict.carbon
+
+import Other;
+// CHECK:STDERR: fail_conflict.carbon:[[@LINE+7]]:1: ERROR: Duplicate name being declared in the same scope.
+// CHECK:STDERR: namespace Other;
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~
+// CHECK:STDERR: fail_conflict.carbon:[[@LINE-4]]:1: Name is previously declared here.
+// CHECK:STDERR: import Other;
+// CHECK:STDERR: ^~~~~~
+// CHECK:STDERR:
+namespace Other;
+// CHECK:STDERR: fail_conflict.carbon:[[@LINE+9]]:10: ERROR: Imported packages cannot be used for declarations.
+// CHECK:STDERR: fn Other.Nested.F();
+// CHECK:STDERR:          ^~~~~~
+// CHECK:STDERR: fail_conflict.carbon:[[@LINE-12]]:1: In import.
+// CHECK:STDERR: import Other;
+// CHECK:STDERR: ^~~~~~
+// CHECK:STDERR: fn.carbon:3:1: Package imported here.
+// CHECK:STDERR: namespace Nested;
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~
+fn Other.Nested.F();
+
+// CHECK:STDOUT: --- fn.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: imports {
+// CHECK:STDOUT:   %Core: <namespace> = namespace file.%Core.import, [template] {
+// CHECK:STDOUT:     import Core//prelude
+// CHECK:STDOUT:     import Core//prelude/operators
+// CHECK:STDOUT:     import Core//prelude/types
+// CHECK:STDOUT:     import Core//prelude/operators/arithmetic
+// CHECK:STDOUT:     import Core//prelude/operators/as
+// CHECK:STDOUT:     import Core//prelude/operators/bitwise
+// CHECK:STDOUT:     import Core//prelude/operators/comparison
+// CHECK:STDOUT:     import Core//prelude/types/bool
+// CHECK:STDOUT:   }
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Core = imports.%Core
+// CHECK:STDOUT:     .Nested = %Nested
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Core.import = import Core
+// CHECK:STDOUT:   %Nested: <namespace> = namespace [template] {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- fail_conflict.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %F.type: type = fn_type @F [template]
+// CHECK:STDOUT:   %.1: type = tuple_type () [template]
+// CHECK:STDOUT:   %F: %F.type = struct_value () [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: imports {
+// CHECK:STDOUT:   %Core: <namespace> = namespace file.%Core.import, [template] {
+// CHECK:STDOUT:     import Core//prelude
+// CHECK:STDOUT:     import Core//prelude/operators
+// CHECK:STDOUT:     import Core//prelude/types
+// CHECK:STDOUT:     import Core//prelude/operators/arithmetic
+// CHECK:STDOUT:     import Core//prelude/operators/as
+// CHECK:STDOUT:     import Core//prelude/operators/bitwise
+// CHECK:STDOUT:     import Core//prelude/operators/comparison
+// CHECK:STDOUT:     import Core//prelude/types/bool
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Other: <namespace> = namespace file.%Other.import, [template] {
+// CHECK:STDOUT:     .Nested = %Nested
+// CHECK:STDOUT:     import Other//default
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %import_ref: <namespace> = import_ref Other//default, inst+3, loaded
+// CHECK:STDOUT:   %Nested: <namespace> = namespace %import_ref, [template] {
+// CHECK:STDOUT:     .F = file.%F.decl
+// CHECK:STDOUT:     import Other//default
+// CHECK:STDOUT:   }
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Core = imports.%Core
+// CHECK:STDOUT:     .Other = imports.%Other
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Core.import = import Core
+// CHECK:STDOUT:   %Other.import = import Other
+// CHECK:STDOUT:   %Other: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Nested = imports.%Nested
+// CHECK:STDOUT:     import Other//default
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %F.decl: %F.type = fn_decl @F [template = constants.%F] {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F();
+// CHECK:STDOUT: