Skip to content

Commit

Permalink
Apply is_closed_import to imported namespaces (#4312)
Browse files Browse the repository at this point in the history 
Spin off from
#4294 (comment)

---------

Co-authored-by: Richard Smith <[email protected]>
Co-authored-by: Carbon Infra Bot <[email protected]>
  • Loading branch information
@dwblaikie @zygoloid @CarbonInfraBot
3 people authored Sep 17, 2024
1 parent 7f930d0 commit ff1cc43
Show file tree
Hide file tree
Showing 2 changed files with 134 additions and 18 deletions.
45 changes: 27 additions & 18 deletions toolchain/check/import.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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());
Expand Down Expand Up @@ -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(
Expand All @@ -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.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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});
Expand Down Expand Up @@ -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(
Expand Down
107 changes: 107 additions & 0 deletions toolchain/check/testdata/namespace/fail_conflict_imported_namespace_nested.carbon
View file
Original file line number Diff line number Diff line change
@@ -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:

0 comments on commit ff1cc43

Please sign in to comment.