diff --git a/toolchain/check/cpp/import.cpp b/toolchain/check/cpp/import.cpp index ee3be35938a8..0cb490715f0b 100644 --- a/toolchain/check/cpp/import.cpp +++ b/toolchain/check/cpp/import.cpp @@ -150,6 +150,22 @@ auto ImportCpp(Context& context, } } +// Returns whether the current context has any C++ imports. If not, produces a +// suitable diagnostic. +static auto CheckForCppContextForIndirectImport(Context& context, + SemIR::LocId loc_id) -> bool { + // TODO: We should perform cross-file imports by importing the C++ AST. For + // now we require the C++ declaration to already be imported into the + // destination file, and find the corresponding declaration there and import + // that. + if (!context.cpp_context()) { + context.TODO( + loc_id, "indirect import of C++ declaration with no direct Cpp import"); + return false; + } + return true; +} + // NOLINTNEXTLINE(misc-no-recursion) static auto FindCorrespondingType(Context& context, SemIR::LocId loc_id, clang::QualType type) -> clang::QualType; @@ -236,6 +252,29 @@ static auto FindCorrespondingDecl(Context& context, SemIR::LocId loc_id, return nullptr; } +auto FindCorrespondingClangDeclKey(Context& context, SemIR::LocId loc_id, + const SemIR::File& file, + SemIR::ClangDeclId clang_decl_id) + -> std::optional { + if (!CheckForCppContextForIndirectImport(context, loc_id)) { + return std::nullopt; + } + CARBON_CHECK(clang_decl_id.has_value()); + auto key = file.clang_decls().Get(clang_decl_id).key; + const auto* decl = key.decl; + auto* corresponding = FindCorrespondingDecl(context, loc_id, decl); + if (!corresponding) { + // TODO: This needs a proper diagnostic. + context.TODO( + loc_id, + "use of imported C++ declaration with no corresponding local import"); + return std::nullopt; + } + + key.decl = corresponding; + return key; +} + // Given a type in some C++ AST which is *not* expected to be `context`, // find the corresponding type in `context`, if there is one. // NOLINTNEXTLINE(misc-no-recursion) @@ -277,23 +316,15 @@ auto ImportCppDeclFromFile(Context& context, SemIR::LocId loc_id, const SemIR::File& file, SemIR::ClangDeclId clang_decl_id) -> SemIR::ConstantId { - CARBON_CHECK(clang_decl_id.has_value()); - auto key = file.clang_decls().Get(clang_decl_id).key; - const auto* decl = key.decl; - auto* corresponding = FindCorrespondingDecl(context, loc_id, decl); - if (!corresponding) { - // TODO: This needs a proper diagnostic. - context.TODO( - loc_id, - "use of imported C++ declaration with no corresponding local import"); + auto key = + FindCorrespondingClangDeclKey(context, loc_id, file, clang_decl_id); + if (!key) { return SemIR::ErrorInst::ConstantId; } - - key.decl = corresponding; - auto imported_inst_id = ImportCppDecl(context, loc_id, key); + auto imported_inst_id = ImportCppDecl(context, loc_id, *key); auto imported_const_id = context.constant_values().Get(imported_inst_id); if (!imported_const_id.is_constant()) { - context.TODO(loc_id, "imported C++ declant is not constant"); + context.TODO(loc_id, "imported C++ declaration is not constant"); return SemIR::ErrorInst::ConstantId; } return imported_const_id; @@ -302,13 +333,7 @@ auto ImportCppDeclFromFile(Context& context, SemIR::LocId loc_id, auto ImportCppConstantFromFile(Context& context, SemIR::LocId loc_id, const SemIR::File& file, SemIR::InstId inst_id) -> SemIR::ConstantId { - // TODO: We should perform cross-file imports by importing the C++ AST. For - // now we require the C++ declaration to already be imported into the - // destination file, and find the corresponding declaration there and import - // that. - if (!context.cpp_context()) { - context.TODO( - loc_id, "indirect import of C++ declaration with no direct Cpp import"); + if (!CheckForCppContextForIndirectImport(context, loc_id)) { return SemIR::ErrorInst::ConstantId; } diff --git a/toolchain/check/cpp/import.h b/toolchain/check/cpp/import.h index 1cbce9749124..096f1fb19095 100644 --- a/toolchain/check/cpp/import.h +++ b/toolchain/check/cpp/import.h @@ -27,6 +27,14 @@ auto ImportCpp(Context& context, llvm::LLVMContext* llvm_context, std::shared_ptr invocation) -> void; +// Given a clang declaration ID that was previously imported into another file, +// returns the corresponding clang declaration key in the current context. +// Produces an error and returns nullopt on failure. +auto FindCorrespondingClangDeclKey(Context& context, SemIR::LocId loc_id, + const SemIR::File& file, + SemIR::ClangDeclId clang_decl_id) + -> std::optional; + // Imports a declaration into the current context that was previously imported // into another file. auto ImportCppDeclFromFile(Context& context, SemIR::LocId loc_id, diff --git a/toolchain/check/import_ref.cpp b/toolchain/check/import_ref.cpp index 6c274e1e1da9..a7f1c95c9271 100644 --- a/toolchain/check/import_ref.cpp +++ b/toolchain/check/import_ref.cpp @@ -3699,11 +3699,24 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver, 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); + auto& local_scope = + resolver.local_name_scopes().Get(namespace_decl.name_scope_id); // Namespaces from this package are eagerly imported, so anything we load here // must be a closed import. - resolver.local_name_scopes() - .Get(namespace_decl.name_scope_id) - .set_is_closed_import(true); + local_scope.set_is_closed_import(true); + + // If this was a C++ namespace, connect it to the corresponding C++ + // declaration in this file. + if (name_scope.is_cpp_scope()) { + if (auto key = FindCorrespondingClangDeclKey( + resolver.local_context(), SemIR::LocId(inst_id), + resolver.import_ir(), name_scope.clang_decl_context_id())) { + auto clang_decl_id = resolver.local_context().clang_decls().Add( + {.key = *key, .inst_id = inst_id}); + local_scope.set_clang_decl_context_id(clang_decl_id, true); + } + } + auto namespace_const_id = ReplacePlaceholderImportedInst(resolver, inst_id, namespace_decl); return {.const_id = namespace_const_id}; diff --git a/toolchain/check/testdata/interop/cpp/basics/import/import.carbon b/toolchain/check/testdata/interop/cpp/basics/import/import.carbon index 72c102baa6a4..e858e466da08 100644 --- a/toolchain/check/testdata/interop/cpp/basics/import/import.carbon +++ b/toolchain/check/testdata/interop/cpp/basics/import/import.carbon @@ -10,41 +10,6 @@ // TIP: To dump output, run: // TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/interop/cpp/basics/import/import.carbon -// ============================================================================ -// Import C++ namespace indirectly -// ============================================================================ - -// --- namespace.h - -namespace MyNamespace { - -class MyClass {}; - -} // namespace MyNamespace - -// --- namespace_api.carbon - -library "[[@TEST_NAME]]"; - -import Cpp library "namespace.h"; - -alias MyNamespaceAlias = Cpp.MyNamespace; - -// --- fail_import_namespace_api.carbon - -library "[[@TEST_NAME]]"; - -import library "namespace_api"; - -fn F() { - // Imports are not implicitly re-exported, so lookup is expected to fail. - // CHECK:STDERR: fail_import_namespace_api.carbon:[[@LINE+4]]:17: error: member name `MyClass` not found in `Cpp.MyNamespace` [MemberNameNotFoundInInstScope] - // CHECK:STDERR: var unused x: MyNamespaceAlias.MyClass; - // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~ - // CHECK:STDERR: - var unused x: MyNamespaceAlias.MyClass; -} - // ============================================================================ // Import C++ struct indirectly // ============================================================================ diff --git a/toolchain/check/testdata/interop/cpp/namespace/import.carbon b/toolchain/check/testdata/interop/cpp/namespace/import.carbon index 08f9a7d5f692..f6ffa9c465f0 100644 --- a/toolchain/check/testdata/interop/cpp/namespace/import.carbon +++ b/toolchain/check/testdata/interop/cpp/namespace/import.carbon @@ -16,7 +16,10 @@ // --- single.h -namespace my_namespace { void foo(); } +namespace my_namespace { + void foo(); + void bar(); +} // --- import_single.carbon @@ -233,6 +236,135 @@ fn Use(y: Cpp.Y) -> i32 { return y.x.n; } +// ============================================================================ +// Import C++ namespace indirectly +// ============================================================================ + +// --- namespace_alias.carbon + +library "[[@TEST_NAME]]"; + +import Cpp library "single.h"; + +alias MyNamespaceAlias = Cpp.my_namespace; + +fn CallFoo() { + MyNamespaceAlias.foo(); +} + +// --- fail_indirect_namespace_no_import.carbon + +library "[[@TEST_NAME]]"; + +// CHECK:STDERR: fail_indirect_namespace_no_import.carbon:[[@LINE+8]]:1: in import [InImport] +// CHECK:STDERR: namespace_alias.carbon:4:1: error: semantics TODO: `indirect import of C++ declaration with no direct Cpp import` [SemanticsTodo] +// CHECK:STDERR: import Cpp library "single.h"; +// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +// CHECK:STDERR: +// CHECK:STDERR: fail_indirect_namespace_no_import.carbon:[[@LINE+3]]:1: in import [InImport] +// CHECK:STDERR: namespace_alias.carbon: error: semantics TODO: `indirect import of C++ declaration with no direct Cpp import` [SemanticsTodo] +// CHECK:STDERR: +import library "namespace_alias"; + +fn F() { + // Imports are not implicitly re-exported, so lookup is expected to fail, + // regardless of whether a lookup was performed in the imported file. + // CHECK:STDERR: fail_indirect_namespace_no_import.carbon:[[@LINE+4]]:3: error: member name `foo` not found in `Cpp.my_namespace` [MemberNameNotFoundInInstScope] + // CHECK:STDERR: MyNamespaceAlias.foo(); + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: + MyNamespaceAlias.foo(); + + // CHECK:STDERR: fail_indirect_namespace_no_import.carbon:[[@LINE+4]]:3: error: member name `bar` not found in `Cpp.my_namespace` [MemberNameNotFoundInInstScope] + // CHECK:STDERR: MyNamespaceAlias.bar(); + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: + MyNamespaceAlias.bar(); +} + +// --- fail_indirect_namespace_no_local_decl.carbon + +library "[[@TEST_NAME]]"; + +// CHECK:STDERR: fail_indirect_namespace_no_local_decl.carbon:[[@LINE+3]]:1: in import [InImport] +// CHECK:STDERR: namespace_alias.carbon: error: semantics TODO: `use of imported C++ declaration with no corresponding local import` [SemanticsTodo] +// CHECK:STDERR: +import library "namespace_alias"; +import Cpp; + +fn F() { + // Imports are not implicitly re-exported, so name lookup is expected to + // fail, regardless of whether a lookup was performed in the imported file. + // CHECK:STDERR: fail_indirect_namespace_no_local_decl.carbon:[[@LINE+4]]:3: error: member name `foo` not found in `Cpp.my_namespace` [MemberNameNotFoundInInstScope] + // CHECK:STDERR: MyNamespaceAlias.foo(); + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: + MyNamespaceAlias.foo(); + + // CHECK:STDERR: fail_indirect_namespace_no_local_decl.carbon:[[@LINE+4]]:3: error: member name `bar` not found in `Cpp.my_namespace` [MemberNameNotFoundInInstScope] + // CHECK:STDERR: MyNamespaceAlias.bar(); + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: + MyNamespaceAlias.bar(); +} + +// --- indirect_namespace_same_contents.carbon + +library "[[@TEST_NAME]]"; + +import library "namespace_alias"; +import Cpp library "single.h"; + +fn F() { + // OK, MyNamespaceAlias is an alias for Cpp.my_namespace. + MyNamespaceAlias.foo(); + MyNamespaceAlias.bar(); +} + +// --- fail_indirect_namespace_different_contents_invalid.carbon + +library "[[@TEST_NAME]]"; + +import library "namespace_alias"; +import Cpp; + +inline Cpp ''' +namespace my_namespace { + void bar(); +} +'''; + +fn F() { + // Cpp imports are not implicitly re-exported, so name lookup is expected to + // fail. + // CHECK:STDERR: fail_indirect_namespace_different_contents_invalid.carbon:[[@LINE+4]]:3: error: member name `foo` not found in `Cpp.my_namespace` [MemberNameNotFoundInInstScope] + // CHECK:STDERR: MyNamespaceAlias.foo(); + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: + MyNamespaceAlias.foo(); +} + +// --- indirect_namespace_different_contents.carbon + +library "[[@TEST_NAME]]"; + +import library "namespace_alias"; +import Cpp; + +inline Cpp ''' +namespace my_namespace { + void bar(); + void baz(); +} +'''; + +fn F() { + // OK, bar is visible here and is found in MyNamespaceAlias, which is an alias + // for Cpp.my_namespace. + MyNamespaceAlias.bar(); + MyNamespaceAlias.baz(); +} + // CHECK:STDOUT: --- import_single.carbon // CHECK:STDOUT: // CHECK:STDOUT: constants {