From 4de92656dd851faa25812fb7c83944cc48fb2065 Mon Sep 17 00:00:00 2001 From: Gabor Horvath Date: Mon, 7 Jul 2025 15:47:02 +0100 Subject: [PATCH] [cxx-interop] Fix unqualified name lookup failure When MemberImportVisibility is enabled we failed to find certain base methods from the extensions when said base methods are imported from C++. rdar://154887575 --- include/swift/AST/ClangModuleLoader.h | 4 +-- include/swift/ClangImporter/ClangImporter.h | 2 +- lib/AST/Decl.cpp | 16 +++++++++++- lib/ClangImporter/ClangImporter.cpp | 25 +++++++++++-------- lib/ClangImporter/ImporterImpl.h | 6 ++--- .../inheritance/Inputs/inherited-lookup.h | 9 +++++++ .../inherited-lookup-executable.swift | 1 + ...erited-lookup-memberimportvisibility.swift | 22 ++++++++++++++++ 8 files changed, 67 insertions(+), 18 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 0901db0085cbe..d6a07148660d3 100644 --- a/include/swift/AST/ClangModuleLoader.h +++ b/include/swift/AST/ClangModuleLoader.h @@ -216,8 +216,8 @@ class ClangModuleLoader : public ModuleLoader { DeclContext *newContext, ClangInheritanceInfo inheritance) = 0; - /// Checks if \param decl is the original method or a clone from a base class - virtual bool isClonedMemberDecl(ValueDecl *decl) = 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. diff --git a/include/swift/ClangImporter/ClangImporter.h b/include/swift/ClangImporter/ClangImporter.h index 61eb28fbb5f1b..6abbdb7855114 100644 --- a/include/swift/ClangImporter/ClangImporter.h +++ b/include/swift/ClangImporter/ClangImporter.h @@ -648,7 +648,7 @@ class ClangImporter final : public ClangModuleLoader { ValueDecl *importBaseMemberDecl(ValueDecl *decl, DeclContext *newContext, ClangInheritanceInfo inheritance) override; - bool isClonedMemberDecl(ValueDecl *decl) override; + ValueDecl *getOriginalForClonedMember(const ValueDecl *decl) override; /// Emits diagnostics for any declarations named name /// whose direct declaration context is a TU. diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 2ba6baf3a7461..41f408c13fdb7 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -910,6 +910,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; @@ -917,8 +928,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 629f74e7814f2..d50e839b0cf99 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -5524,10 +5524,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; @@ -6317,7 +6318,7 @@ TinyPtrVector ClangRecordMemberLookup::evaluate( auto namedMember = dyn_cast(member); if (!namedMember || !namedMember->hasName() || namedMember->getName().getBaseName() != name || - clangModuleLoader->isClonedMemberDecl(namedMember)) + clangModuleLoader->getOriginalForClonedMember(namedMember)) continue; auto *imported = clangModuleLoader->importBaseMemberDecl( @@ -7670,22 +7671,24 @@ ValueDecl *ClangImporter::Implementation::importBaseMemberDecl( if (known == clonedBaseMembers.end()) { ValueDecl *cloned = cloneBaseMemberDecl(decl, newContext, inheritance); known = clonedBaseMembers.insert({key, cloned}).first; - clonedMembers.insert(cloned); + clonedMembers.insert(std::make_pair(cloned, decl)); } return known->second; } -bool ClangImporter::Implementation::isClonedMemberDecl(ValueDecl *decl) { +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); - return result != clonedMembers.end(); + if (result != clonedMembers.end()) + return result->getSecond(); } - return false; + return nullptr; } size_t ClangImporter::Implementation::getImportedBaseMemberDeclArity( @@ -7704,8 +7707,8 @@ ClangImporter::importBaseMemberDecl(ValueDecl *decl, DeclContext *newContext, return Impl.importBaseMemberDecl(decl, newContext, inheritance); } -bool ClangImporter::isClonedMemberDecl(ValueDecl *decl) { - return Impl.isClonedMemberDecl(decl); +ValueDecl *ClangImporter::getOriginalForClonedMember(const ValueDecl *decl) { + return Impl.getOriginalForClonedMember(decl); } void ClangImporter::diagnoseTopLevelValue(const DeclName &name) { diff --git a/lib/ClangImporter/ImporterImpl.h b/lib/ClangImporter/ImporterImpl.h index a9645a9aff202..9e293463a560c 100644 --- a/lib/ClangImporter/ImporterImpl.h +++ b/lib/ClangImporter/ImporterImpl.h @@ -691,8 +691,8 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation llvm::DenseMap, ValueDecl *> clonedBaseMembers; - // Store all methods that result from cloning a base member - llvm::DenseSet clonedMembers; + // Map all cloned methods back to the original member + llvm::DenseMap clonedMembers; public: llvm::DenseMap defaultArgGenerators; @@ -702,7 +702,7 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation ValueDecl *importBaseMemberDecl(ValueDecl *decl, DeclContext *newContext, ClangInheritanceInfo inheritance); - bool isClonedMemberDecl(ValueDecl *decl); + ValueDecl *getOriginalForClonedMember(const ValueDecl *decl); static size_t getImportedBaseMemberDeclArity(const ValueDecl *valueDecl); 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 b4136ef715db5..d6292fc25cd20 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()