From 0b886b2b30128289cbfd67634386b0115216946b Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Tue, 10 Dec 2024 04:11:33 +0000 Subject: [PATCH] Import enclosing namespaces when importing an entity. Ensures that we can properly name entities imported indirectly. --- toolchain/check/import_ref.cpp | 46 +++-- .../namespace/imported_indirect.carbon | 174 +++++++++++++++++- toolchain/check/testdata/struct/import.carbon | 4 +- toolchain/check/testdata/tuple/import.carbon | 4 +- 4 files changed, 209 insertions(+), 19 deletions(-) diff --git a/toolchain/check/import_ref.cpp b/toolchain/check/import_ref.cpp index bbf8b12a6e3bc..db0ef9452a390 100644 --- a/toolchain/check/import_ref.cpp +++ b/toolchain/check/import_ref.cpp @@ -1082,18 +1082,7 @@ static auto GetLocalNameScopeId(ImportRefResolver& resolver, } // Get the constant value for the scope. - auto const_id = SemIR::ConstantId::Invalid; - CARBON_KIND_SWITCH(*inst) { - case SemIR::Namespace::Kind: - // If the namespace has already been imported, we can use its constant. - // However, if it hasn't, we use Invalid instead of adding it to the - // work stack. That's expected to be okay when resolving references. - const_id = resolver.local_constant_values_for_import_insts().Get(inst_id); - break; - - default: - const_id = GetLocalConstantId(resolver, inst_id); - } + auto const_id = GetLocalConstantId(resolver, inst_id); if (!const_id.is_valid()) { return SemIR::NameScopeId::Invalid; } @@ -2280,6 +2269,36 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver, .bit_width_id = bit_width_id}); } +static auto TryResolveTypedInst(ImportRefResolver& resolver, + SemIR::Namespace inst, + SemIR::InstId import_inst_id) -> ResolveResult { + const auto& name_scope = + resolver.import_name_scopes().Get(inst.name_scope_id); + auto parent_scope_id = + GetLocalNameScopeId(resolver, name_scope.parent_scope_id()); + + if (resolver.HasNewWork()) { + return ResolveResult::Retry(); + } + + auto namespace_type_id = resolver.local_context().GetSingletonType( + SemIR::NamespaceType::SingletonInstId); + auto namespace_decl = + SemIR::Namespace{.type_id = namespace_type_id, + .name_scope_id = SemIR::NameScopeId::Invalid, + .import_id = SemIR::AbsoluteInstId::Invalid}; + auto inst_id = resolver.local_context().AddPlaceholderInstInNoBlock( + resolver.local_context().MakeImportedLocAndInst( + AddImportIRInst(resolver, import_inst_id), namespace_decl)); + + auto name_id = GetLocalNameId(resolver, name_scope.name_id()); + namespace_decl.name_scope_id = + resolver.local_name_scopes().Add(inst_id, name_id, parent_scope_id); + resolver.local_context().ReplaceInstBeforeConstantUse(inst_id, + namespace_decl); + return {.const_id = resolver.local_constant_values().Get(inst_id)}; +} + static auto TryResolveTypedInst(ImportRefResolver& resolver, SemIR::PointerType inst) -> ResolveResult { CARBON_CHECK(inst.type_id == SemIR::TypeType::SingletonTypeId); @@ -2519,6 +2538,9 @@ static auto TryResolveInstCanonical(ImportRefResolver& resolver, case CARBON_KIND(SemIR::IntType inst): { return TryResolveTypedInst(resolver, inst); } + case CARBON_KIND(SemIR::Namespace inst): { + return TryResolveTypedInst(resolver, inst, inst_id); + } case CARBON_KIND(SemIR::PointerType inst): { return TryResolveTypedInst(resolver, inst); } diff --git a/toolchain/check/testdata/namespace/imported_indirect.carbon b/toolchain/check/testdata/namespace/imported_indirect.carbon index 55f62d06d9df8..14fcca5ba8d0e 100644 --- a/toolchain/check/testdata/namespace/imported_indirect.carbon +++ b/toolchain/check/testdata/namespace/imported_indirect.carbon @@ -14,6 +14,8 @@ package Same library "[[@TEST_NAME]]"; namespace A; +class A.C; + // --- b.carbon package Same library "[[@TEST_NAME]]"; @@ -21,6 +23,8 @@ import library "a"; namespace A.B; +fn F() -> A.C; + // --- c.carbon package Same library "[[@TEST_NAME]]"; @@ -42,8 +46,53 @@ import library "d"; var e: () = A.B.C.D(); +// --- fail_named_indirectly_same_package.carbon + +package Same library "[[@TEST_NAME]]"; + +import library "b"; + +// CHECK:STDERR: fail_named_indirectly_same_package.carbon:[[@LINE+13]]:10: error: function returns incomplete type `A.C` [IncompleteTypeInFunctionReturnType] +// CHECK:STDERR: fn G() { F(); } +// CHECK:STDERR: ^ +// CHECK:STDERR: fail_named_indirectly_same_package.carbon:[[@LINE-5]]:1: in import [InImport] +// CHECK:STDERR: b.carbon:3:1: in import [InImport] +// CHECK:STDERR: a.carbon:6:1: note: class was forward declared here [ClassForwardDeclaredHere] +// CHECK:STDERR: class A.C; +// CHECK:STDERR: ^~~~~~~~~~ +// CHECK:STDERR: fail_named_indirectly_same_package.carbon:[[@LINE-10]]:1: in import [InImport] +// CHECK:STDERR: b.carbon:7:8: note: return type declared here [IncompleteReturnTypeHere] +// CHECK:STDERR: fn F() -> A.C; +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: +fn G() { F(); } + +// --- fail_named_indirectly_different_package.carbon + +package Other library "[[@TEST_NAME]]"; + +import Same library "b"; + +// CHECK:STDERR: fail_named_indirectly_different_package.carbon:[[@LINE+12]]:10: error: function returns incomplete type `Same.A.C` [IncompleteTypeInFunctionReturnType] +// CHECK:STDERR: fn G() { Same.F(); } +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: fail_named_indirectly_different_package.carbon:[[@LINE-5]]:1: in import [InImport] +// CHECK:STDERR: b.carbon:3:1: in import [InImport] +// CHECK:STDERR: a.carbon:6:1: note: class was forward declared here [ClassForwardDeclaredHere] +// CHECK:STDERR: class A.C; +// CHECK:STDERR: ^~~~~~~~~~ +// CHECK:STDERR: fail_named_indirectly_different_package.carbon:[[@LINE-10]]:1: in import [InImport] +// CHECK:STDERR: b.carbon:7:8: note: return type declared here [IncompleteReturnTypeHere] +// CHECK:STDERR: fn F() -> A.C; +// CHECK:STDERR: ^~~~~~ +fn G() { Same.F(); } + // CHECK:STDOUT: --- a.carbon // CHECK:STDOUT: +// CHECK:STDOUT: constants { +// CHECK:STDOUT: %C: type = class_type @C [template] +// CHECK:STDOUT: } +// CHECK:STDOUT: // CHECK:STDOUT: imports { // CHECK:STDOUT: %Core: = namespace file.%Core.import, [template] { // CHECK:STDOUT: import Core//prelude @@ -57,16 +106,29 @@ var e: () = A.B.C.D(); // CHECK:STDOUT: .A = %A // CHECK:STDOUT: } // CHECK:STDOUT: %Core.import = import Core -// CHECK:STDOUT: %A: = namespace [template] {} +// CHECK:STDOUT: %A: = namespace [template] { +// CHECK:STDOUT: .C = %C.decl +// CHECK:STDOUT: } +// CHECK:STDOUT: %C.decl: type = class_decl @C [template = constants.%C] {} {} // CHECK:STDOUT: } // CHECK:STDOUT: +// CHECK:STDOUT: class @C; +// CHECK:STDOUT: // CHECK:STDOUT: --- b.carbon // CHECK:STDOUT: +// CHECK:STDOUT: constants { +// CHECK:STDOUT: %C: type = class_type @C [template] +// CHECK:STDOUT: %F.type: type = fn_type @F [template] +// CHECK:STDOUT: %F: %F.type = struct_value () [template] +// CHECK:STDOUT: } +// CHECK:STDOUT: // CHECK:STDOUT: imports { -// CHECK:STDOUT: %import_ref: = import_ref Same//a, A, loaded -// CHECK:STDOUT: %A: = namespace %import_ref, [template] { +// CHECK:STDOUT: %import_ref.1: = import_ref Same//a, A, loaded +// CHECK:STDOUT: %A: = namespace %import_ref.1, [template] { +// CHECK:STDOUT: .C = %import_ref.2 // CHECK:STDOUT: .B = file.%B // CHECK:STDOUT: } +// CHECK:STDOUT: %import_ref.2: type = import_ref Same//a, C, loaded [template = constants.%C] // CHECK:STDOUT: %Core: = namespace file.%Core.import, [template] { // CHECK:STDOUT: import Core//prelude // CHECK:STDOUT: import Core//prelude/... @@ -77,12 +139,26 @@ var e: () = A.B.C.D(); // CHECK:STDOUT: package: = namespace [template] { // CHECK:STDOUT: .A = imports.%A // CHECK:STDOUT: .Core = imports.%Core +// CHECK:STDOUT: .F = %F.decl // CHECK:STDOUT: } // CHECK:STDOUT: %Core.import = import Core // CHECK:STDOUT: %default.import = import // CHECK:STDOUT: %B: = namespace [template] {} +// CHECK:STDOUT: %F.decl: %F.type = fn_decl @F [template = constants.%F] { +// CHECK:STDOUT: %return.patt: %C = return_slot_pattern +// CHECK:STDOUT: %return.param_patt: %C = out_param_pattern %return.patt, runtime_param0 +// CHECK:STDOUT: } { +// CHECK:STDOUT: %A.ref: = name_ref A, imports.%A [template = imports.%A] +// CHECK:STDOUT: %C.ref: type = name_ref C, imports.%import_ref.2 [template = constants.%C] +// CHECK:STDOUT: %return.param: ref %C = out_param runtime_param0 +// CHECK:STDOUT: %return: ref %C = return_slot %return.param +// CHECK:STDOUT: } // CHECK:STDOUT: } // CHECK:STDOUT: +// CHECK:STDOUT: class @C [from "a.carbon"]; +// CHECK:STDOUT: +// CHECK:STDOUT: fn @F() -> %C; +// CHECK:STDOUT: // CHECK:STDOUT: --- c.carbon // CHECK:STDOUT: // CHECK:STDOUT: imports { @@ -90,6 +166,7 @@ var e: () = A.B.C.D(); // CHECK:STDOUT: %A: = namespace %import_ref.1, [template] { // CHECK:STDOUT: .B = %B // CHECK:STDOUT: } +// CHECK:STDOUT: %import_ref.3 = import_ref Same//b, F, unloaded // CHECK:STDOUT: %Core: = namespace file.%Core.import, [template] { // CHECK:STDOUT: import Core//prelude // CHECK:STDOUT: import Core//prelude/... @@ -99,6 +176,7 @@ var e: () = A.B.C.D(); // CHECK:STDOUT: file { // CHECK:STDOUT: package: = namespace [template] { // CHECK:STDOUT: .A = imports.%A +// CHECK:STDOUT: .F = imports.%import_ref.3 // CHECK:STDOUT: .Core = imports.%Core // CHECK:STDOUT: } // CHECK:STDOUT: %Core.import = import Core @@ -194,3 +272,93 @@ var e: () = A.B.C.D(); // CHECK:STDOUT: return // CHECK:STDOUT: } // CHECK:STDOUT: +// CHECK:STDOUT: --- fail_named_indirectly_same_package.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: constants { +// CHECK:STDOUT: %G.type: type = fn_type @G [template] +// CHECK:STDOUT: %G: %G.type = struct_value () [template] +// CHECK:STDOUT: %F.type: type = fn_type @F [template] +// CHECK:STDOUT: %F: %F.type = struct_value () [template] +// CHECK:STDOUT: %C: type = class_type @C [template] +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: imports { +// CHECK:STDOUT: %import_ref.1: = import_ref Same//b, A, loaded +// CHECK:STDOUT: %A: = namespace %import_ref.1, [template] { +// CHECK:STDOUT: .B = %B +// CHECK:STDOUT: } +// CHECK:STDOUT: %import_ref.3: %F.type = import_ref Same//b, F, loaded [template = constants.%F] +// CHECK:STDOUT: %Core: = namespace file.%Core.import, [template] { +// CHECK:STDOUT: import Core//prelude +// CHECK:STDOUT: import Core//prelude/... +// CHECK:STDOUT: } +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .A = imports.%A +// CHECK:STDOUT: .F = imports.%import_ref.3 +// CHECK:STDOUT: .Core = imports.%Core +// CHECK:STDOUT: .G = %G.decl +// CHECK:STDOUT: } +// CHECK:STDOUT: %Core.import = import Core +// CHECK:STDOUT: %default.import = import +// CHECK:STDOUT: %G.decl: %G.type = fn_decl @G [template = constants.%G] {} {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: class @C [from "b.carbon"]; +// CHECK:STDOUT: +// CHECK:STDOUT: fn @G() { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: %F.ref: %F.type = name_ref F, imports.%import_ref.3 [template = constants.%F] +// CHECK:STDOUT: %F.call: init = call %F.ref() +// CHECK:STDOUT: return +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @F() -> %C [from "b.carbon"]; +// CHECK:STDOUT: +// CHECK:STDOUT: --- fail_named_indirectly_different_package.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: constants { +// CHECK:STDOUT: %G.type: type = fn_type @G [template] +// CHECK:STDOUT: %G: %G.type = struct_value () [template] +// CHECK:STDOUT: %F.type: type = fn_type @F [template] +// CHECK:STDOUT: %F: %F.type = struct_value () [template] +// CHECK:STDOUT: %C: type = class_type @C [template] +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: imports { +// CHECK:STDOUT: %Core: = namespace file.%Core.import, [template] { +// CHECK:STDOUT: import Core//prelude +// CHECK:STDOUT: import Core//prelude/... +// CHECK:STDOUT: } +// CHECK:STDOUT: %Same: = namespace file.%Same.import, [template] { +// CHECK:STDOUT: .F = %import_ref +// CHECK:STDOUT: import Same//b +// CHECK:STDOUT: } +// CHECK:STDOUT: %import_ref: %F.type = import_ref Same//b, F, loaded [template = constants.%F] +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .Core = imports.%Core +// CHECK:STDOUT: .Same = imports.%Same +// CHECK:STDOUT: .G = %G.decl +// CHECK:STDOUT: } +// CHECK:STDOUT: %Core.import = import Core +// CHECK:STDOUT: %Same.import = import Same +// CHECK:STDOUT: %G.decl: %G.type = fn_decl @G [template = constants.%G] {} {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: class @C [from "b.carbon"]; +// CHECK:STDOUT: +// CHECK:STDOUT: fn @G() { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: %Same.ref: = name_ref Same, imports.%Same [template = imports.%Same] +// CHECK:STDOUT: %F.ref: %F.type = name_ref F, imports.%import_ref [template = constants.%F] +// CHECK:STDOUT: %F.call: init = call %F.ref() +// CHECK:STDOUT: return +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @F() -> %C [from "b.carbon"]; +// CHECK:STDOUT: diff --git a/toolchain/check/testdata/struct/import.carbon b/toolchain/check/testdata/struct/import.carbon index ca557774bf9fd..283efd5cbdf83 100644 --- a/toolchain/check/testdata/struct/import.carbon +++ b/toolchain/check/testdata/struct/import.carbon @@ -43,10 +43,10 @@ var c_bad: C({.c = 1, .d = 2}) = F(); // --- fail_bad_value.impl.carbon impl package Implicit; -// CHECK:STDERR: fail_bad_value.impl.carbon:[[@LINE+6]]:1: error: cannot implicitly convert from `C()` to `C()` [ImplicitAsConversionFailure] +// CHECK:STDERR: fail_bad_value.impl.carbon:[[@LINE+6]]:1: error: cannot implicitly convert from `C()` to `C()` [ImplicitAsConversionFailure] // CHECK:STDERR: var c_bad: C({.a = 3, .b = 4}) = F(); // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -// CHECK:STDERR: fail_bad_value.impl.carbon:[[@LINE+3]]:1: note: type `C()` does not implement interface `ImplicitAs(C())` [MissingImplInMemberAccessNote] +// CHECK:STDERR: fail_bad_value.impl.carbon:[[@LINE+3]]:1: note: type `C()` does not implement interface `Core.ImplicitAs(C())` [MissingImplInMemberAccessNote] // CHECK:STDERR: var c_bad: C({.a = 3, .b = 4}) = F(); // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ var c_bad: C({.a = 3, .b = 4}) = F(); diff --git a/toolchain/check/testdata/tuple/import.carbon b/toolchain/check/testdata/tuple/import.carbon index d9e569e9e9e0a..489dd09509d07 100644 --- a/toolchain/check/testdata/tuple/import.carbon +++ b/toolchain/check/testdata/tuple/import.carbon @@ -45,10 +45,10 @@ var c_bad: C((1, 2, 3)) = F(); impl package Implicit; -// CHECK:STDERR: fail_bad_value.impl.carbon:[[@LINE+6]]:1: error: cannot implicitly convert from `C()` to `C()` [ImplicitAsConversionFailure] +// CHECK:STDERR: fail_bad_value.impl.carbon:[[@LINE+6]]:1: error: cannot implicitly convert from `C()` to `C()` [ImplicitAsConversionFailure] // CHECK:STDERR: var c_bad: C((3, 4)) = F(); // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~ -// CHECK:STDERR: fail_bad_value.impl.carbon:[[@LINE+3]]:1: note: type `C()` does not implement interface `ImplicitAs(C())` [MissingImplInMemberAccessNote] +// CHECK:STDERR: fail_bad_value.impl.carbon:[[@LINE+3]]:1: note: type `C()` does not implement interface `Core.ImplicitAs(C())` [MissingImplInMemberAccessNote] // CHECK:STDERR: var c_bad: C((3, 4)) = F(); // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~ var c_bad: C((3, 4)) = F();