From b51b58db302867bc7d7d6e995db4b2668b650fe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Horv=C3=A1th?= Date: Tue, 8 Jul 2025 17:53:35 +0100 Subject: [PATCH] [6.2][cxx-interop] Fix unqualified name lookup failure Explanation: C++ interop synthesizes certain forwarding functions in an _ObjC module. This confuses MemberImportVisibility. This patch adds logic to work this around by keeping a mapping between the synthesized and the original function and looks up where the synthesized functions belong to based on the original functions' parent module. Scope: C++ forward interop when MemberImportVisibility is enabled. Issues: rdar://154887575 Original PRs: #82840 Risk: Low, a narrow change makes getModuleContextForNameLookupForCxxDecl more precise, and it is only used with MemberImportVisibility. Testing: Added a compiler test. Reviewers: @egorzhdan, @tshortli, @hnrklssn --- include/swift/AST/ClangModuleLoader.h | 3 ++ include/swift/ClangImporter/ClangImporter.h | 2 ++ lib/AST/Decl.cpp | 16 ++++++++++- lib/ClangImporter/ClangImporter.cpp | 28 ++++++++++++++++--- lib/ClangImporter/ImporterImpl.h | 5 ++++ .../inheritance/Inputs/inherited-lookup.h | 9 ++++++ .../inherited-lookup-executable.swift | 1 + ...erited-lookup-memberimportvisibility.swift | 22 +++++++++++++++ 8 files changed, 81 insertions(+), 5 deletions(-) create mode 100644 test/Interop/Cxx/class/inheritance/inherited-lookup-memberimportvisibility.swift diff --git a/include/swift/AST/ClangModuleLoader.h b/include/swift/AST/ClangModuleLoader.h index 8213540c0a595..d6a07148660d3 100644 --- a/include/swift/AST/ClangModuleLoader.h +++ b/include/swift/AST/ClangModuleLoader.h @@ -216,6 +216,9 @@ class ClangModuleLoader : public ModuleLoader { DeclContext *newContext, ClangInheritanceInfo inheritance) = 0; + /// Returnes the original method if \param decl is a clone from a base class + virtual ValueDecl *getOriginalForClonedMember(const ValueDecl *decl) = 0; + /// Emits diagnostics for any declarations named name /// whose direct declaration context is a TU. virtual void diagnoseTopLevelValue(const DeclName &name) = 0; diff --git a/include/swift/ClangImporter/ClangImporter.h b/include/swift/ClangImporter/ClangImporter.h index a85be39e13389..90d8cddc5c8d9 100644 --- a/include/swift/ClangImporter/ClangImporter.h +++ b/include/swift/ClangImporter/ClangImporter.h @@ -655,6 +655,8 @@ class ClangImporter final : public ClangModuleLoader { ValueDecl *importBaseMemberDecl(ValueDecl *decl, DeclContext *newContext, ClangInheritanceInfo inheritance) override; + ValueDecl *getOriginalForClonedMember(const ValueDecl *decl) override; + /// Emits diagnostics for any declarations named name /// whose direct declaration context is a TU. void diagnoseTopLevelValue(const DeclName &name) override; diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index a87846e79ea15..f96299cf1c09f 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -906,6 +906,17 @@ static ModuleDecl *getModuleContextForNameLookupForCxxDecl(const Decl *decl) { if (!ctx.LangOpts.EnableCXXInterop) return nullptr; + // When we clone members for base classes the cloned members have no + // corresponding Clang nodes. Look up the original imported declaration to + // figure out what Clang module does the cloned member originate from. + bool isClonedMember = false; + if (auto VD = dyn_cast(decl)) + if (auto loader = ctx.getClangModuleLoader()) + if (auto original = loader->getOriginalForClonedMember(VD)) { + isClonedMember = true; + decl = original; + } + if (!decl->hasClangNode()) return nullptr; @@ -913,8 +924,11 @@ static ModuleDecl *getModuleContextForNameLookupForCxxDecl(const Decl *decl) { // We only need to look for the real parent module when the existing parent // is the imported header module. - if (!parentModule->isClangHeaderImportModule()) + if (!parentModule->isClangHeaderImportModule()) { + if (isClonedMember) + return parentModule; return nullptr; + } auto clangModule = decl->getClangDecl()->getOwningModule(); if (!clangModule) diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index 753b51d57a3b5..6ef1d4c47def8 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -5516,10 +5516,11 @@ const clang::CXXMethodDecl *getCalledBaseCxxMethod(FuncDecl *baseMember) { // Construct a Swift method that represents the synthesized C++ method // that invokes the base C++ method. -FuncDecl *synthesizeBaseFunctionDeclCall(ClangImporter &impl, ASTContext &ctx, - NominalTypeDecl *derivedStruct, - NominalTypeDecl *baseStruct, - FuncDecl *baseMember) { +static FuncDecl *synthesizeBaseFunctionDeclCall(ClangImporter &impl, + ASTContext &ctx, + NominalTypeDecl *derivedStruct, + NominalTypeDecl *baseStruct, + FuncDecl *baseMember) { auto *cxxMethod = getCalledBaseCxxMethod(baseMember); if (!cxxMethod) return nullptr; @@ -7661,11 +7662,26 @@ ValueDecl *ClangImporter::Implementation::importBaseMemberDecl( if (known == clonedBaseMembers.end()) { ValueDecl *cloned = cloneBaseMemberDecl(decl, newContext, inheritance); known = clonedBaseMembers.insert({key, cloned}).first; + clonedMembers.insert(std::make_pair(cloned, decl)); } return known->second; } +ValueDecl *ClangImporter::Implementation::getOriginalForClonedMember( + const ValueDecl *decl) { + // If this is a cloned decl, we don't want to reclone it + // Otherwise, we may end up with multiple copies of the same method + if (!decl->hasClangNode()) { + // Skip decls with a clang node as those will never be a clone + auto result = clonedMembers.find(decl); + if (result != clonedMembers.end()) + return result->getSecond(); + } + + return nullptr; +} + size_t ClangImporter::Implementation::getImportedBaseMemberDeclArity( const ValueDecl *valueDecl) { if (auto *func = dyn_cast(valueDecl)) { @@ -7682,6 +7698,10 @@ ClangImporter::importBaseMemberDecl(ValueDecl *decl, DeclContext *newContext, return Impl.importBaseMemberDecl(decl, newContext, inheritance); } +ValueDecl *ClangImporter::getOriginalForClonedMember(const ValueDecl *decl) { + return Impl.getOriginalForClonedMember(decl); +} + void ClangImporter::diagnoseTopLevelValue(const DeclName &name) { Impl.diagnoseTopLevelValue(name); } diff --git a/lib/ClangImporter/ImporterImpl.h b/lib/ClangImporter/ImporterImpl.h index 50cd880db8456..f7a88229ed64a 100644 --- a/lib/ClangImporter/ImporterImpl.h +++ b/lib/ClangImporter/ImporterImpl.h @@ -688,6 +688,9 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation llvm::DenseMap, ValueDecl *> clonedBaseMembers; + // Map all cloned methods back to the original member + llvm::DenseMap clonedMembers; + public: llvm::DenseMap defaultArgGenerators; @@ -696,6 +699,8 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation ValueDecl *importBaseMemberDecl(ValueDecl *decl, DeclContext *newContext, ClangInheritanceInfo inheritance); + ValueDecl *getOriginalForClonedMember(const ValueDecl *decl); + static size_t getImportedBaseMemberDeclArity(const ValueDecl *valueDecl); // Cache for already-specialized function templates and any thunks they may diff --git a/test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h b/test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h index c2bf7ff4fe9f3..b67cbbc1f85e5 100644 --- a/test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h +++ b/test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h @@ -16,3 +16,12 @@ struct IIOne : IOne { struct IIIOne : IIOne { int methodIII(void) const { return -111; } }; + +class Base { +public: + bool baseMethod() const { return true; } +}; + +namespace Bar { +class Derived : public Base {}; +} // namespace Bar diff --git a/test/Interop/Cxx/class/inheritance/inherited-lookup-executable.swift b/test/Interop/Cxx/class/inheritance/inherited-lookup-executable.swift index 664c50381f6f5..0debc35335197 100644 --- a/test/Interop/Cxx/class/inheritance/inherited-lookup-executable.swift +++ b/test/Interop/Cxx/class/inheritance/inherited-lookup-executable.swift @@ -1,6 +1,7 @@ // RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -cxx-interoperability-mode=default) // // REQUIRES: executable_test + import InheritedLookup import StdlibUnittest diff --git a/test/Interop/Cxx/class/inheritance/inherited-lookup-memberimportvisibility.swift b/test/Interop/Cxx/class/inheritance/inherited-lookup-memberimportvisibility.swift new file mode 100644 index 0000000000000..49e62dea469d0 --- /dev/null +++ b/test/Interop/Cxx/class/inheritance/inherited-lookup-memberimportvisibility.swift @@ -0,0 +1,22 @@ +// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -cxx-interoperability-mode=default -enable-upcoming-feature MemberImportVisibility) +// +// REQUIRES: executable_test +// REQUIRES: swift_feature_MemberImportVisibility + +import InheritedLookup +import StdlibUnittest + +var InheritedMemberTestSuite = TestSuite("Test if inherited lookup works") + +extension Bar.Derived { + public func callBase() { + let _ = baseMethod() + } +} + +InheritedMemberTestSuite.test("Look up base methods from extensions") { + let a = Bar.Derived() + a.callBase() +} + +runAllTests()