Skip to content

Commit 4de9265

Browse files
author
Gabor Horvath
committed
[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
1 parent 31408fe commit 4de9265

File tree

8 files changed

+67
-18
lines changed

8 files changed

+67
-18
lines changed

include/swift/AST/ClangModuleLoader.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,8 +216,8 @@ class ClangModuleLoader : public ModuleLoader {
216216
DeclContext *newContext,
217217
ClangInheritanceInfo inheritance) = 0;
218218

219-
/// Checks if \param decl is the original method or a clone from a base class
220-
virtual bool isClonedMemberDecl(ValueDecl *decl) = 0;
219+
/// Returnes the original method if \param decl is a clone from a base class
220+
virtual ValueDecl *getOriginalForClonedMember(const ValueDecl *decl) = 0;
221221

222222
/// Emits diagnostics for any declarations named name
223223
/// whose direct declaration context is a TU.

include/swift/ClangImporter/ClangImporter.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,7 @@ class ClangImporter final : public ClangModuleLoader {
648648
ValueDecl *importBaseMemberDecl(ValueDecl *decl, DeclContext *newContext,
649649
ClangInheritanceInfo inheritance) override;
650650

651-
bool isClonedMemberDecl(ValueDecl *decl) override;
651+
ValueDecl *getOriginalForClonedMember(const ValueDecl *decl) override;
652652

653653
/// Emits diagnostics for any declarations named name
654654
/// whose direct declaration context is a TU.

lib/AST/Decl.cpp

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -910,15 +910,29 @@ static ModuleDecl *getModuleContextForNameLookupForCxxDecl(const Decl *decl) {
910910
if (!ctx.LangOpts.EnableCXXInterop)
911911
return nullptr;
912912

913+
// When we clone members for base classes the cloned members have no
914+
// corresponding Clang nodes. Look up the original imported declaration to
915+
// figure out what Clang module does the cloned member originate from.
916+
bool isClonedMember = false;
917+
if (auto VD = dyn_cast<ValueDecl>(decl))
918+
if (auto loader = ctx.getClangModuleLoader())
919+
if (auto original = loader->getOriginalForClonedMember(VD)) {
920+
isClonedMember = true;
921+
decl = original;
922+
}
923+
913924
if (!decl->hasClangNode())
914925
return nullptr;
915926

916927
auto parentModule = decl->getModuleContext();
917928

918929
// We only need to look for the real parent module when the existing parent
919930
// is the imported header module.
920-
if (!parentModule->isClangHeaderImportModule())
931+
if (!parentModule->isClangHeaderImportModule()) {
932+
if (isClonedMember)
933+
return parentModule;
921934
return nullptr;
935+
}
922936

923937
auto clangModule = decl->getClangDecl()->getOwningModule();
924938
if (!clangModule)

lib/ClangImporter/ClangImporter.cpp

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5524,10 +5524,11 @@ const clang::CXXMethodDecl *getCalledBaseCxxMethod(FuncDecl *baseMember) {
55245524

55255525
// Construct a Swift method that represents the synthesized C++ method
55265526
// that invokes the base C++ method.
5527-
FuncDecl *synthesizeBaseFunctionDeclCall(ClangImporter &impl, ASTContext &ctx,
5528-
NominalTypeDecl *derivedStruct,
5529-
NominalTypeDecl *baseStruct,
5530-
FuncDecl *baseMember) {
5527+
static FuncDecl *synthesizeBaseFunctionDeclCall(ClangImporter &impl,
5528+
ASTContext &ctx,
5529+
NominalTypeDecl *derivedStruct,
5530+
NominalTypeDecl *baseStruct,
5531+
FuncDecl *baseMember) {
55315532
auto *cxxMethod = getCalledBaseCxxMethod(baseMember);
55325533
if (!cxxMethod)
55335534
return nullptr;
@@ -6317,7 +6318,7 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
63176318
auto namedMember = dyn_cast<ValueDecl>(member);
63186319
if (!namedMember || !namedMember->hasName() ||
63196320
namedMember->getName().getBaseName() != name ||
6320-
clangModuleLoader->isClonedMemberDecl(namedMember))
6321+
clangModuleLoader->getOriginalForClonedMember(namedMember))
63216322
continue;
63226323

63236324
auto *imported = clangModuleLoader->importBaseMemberDecl(
@@ -7670,22 +7671,24 @@ ValueDecl *ClangImporter::Implementation::importBaseMemberDecl(
76707671
if (known == clonedBaseMembers.end()) {
76717672
ValueDecl *cloned = cloneBaseMemberDecl(decl, newContext, inheritance);
76727673
known = clonedBaseMembers.insert({key, cloned}).first;
7673-
clonedMembers.insert(cloned);
7674+
clonedMembers.insert(std::make_pair(cloned, decl));
76747675
}
76757676

76767677
return known->second;
76777678
}
76787679

7679-
bool ClangImporter::Implementation::isClonedMemberDecl(ValueDecl *decl) {
7680+
ValueDecl *ClangImporter::Implementation::getOriginalForClonedMember(
7681+
const ValueDecl *decl) {
76807682
// If this is a cloned decl, we don't want to reclone it
76817683
// Otherwise, we may end up with multiple copies of the same method
76827684
if (!decl->hasClangNode()) {
76837685
// Skip decls with a clang node as those will never be a clone
76847686
auto result = clonedMembers.find(decl);
7685-
return result != clonedMembers.end();
7687+
if (result != clonedMembers.end())
7688+
return result->getSecond();
76867689
}
76877690

7688-
return false;
7691+
return nullptr;
76897692
}
76907693

76917694
size_t ClangImporter::Implementation::getImportedBaseMemberDeclArity(
@@ -7704,8 +7707,8 @@ ClangImporter::importBaseMemberDecl(ValueDecl *decl, DeclContext *newContext,
77047707
return Impl.importBaseMemberDecl(decl, newContext, inheritance);
77057708
}
77067709

7707-
bool ClangImporter::isClonedMemberDecl(ValueDecl *decl) {
7708-
return Impl.isClonedMemberDecl(decl);
7710+
ValueDecl *ClangImporter::getOriginalForClonedMember(const ValueDecl *decl) {
7711+
return Impl.getOriginalForClonedMember(decl);
77097712
}
77107713

77117714
void ClangImporter::diagnoseTopLevelValue(const DeclName &name) {

lib/ClangImporter/ImporterImpl.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -691,8 +691,8 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
691691
llvm::DenseMap<std::pair<ValueDecl *, DeclContext *>, ValueDecl *>
692692
clonedBaseMembers;
693693

694-
// Store all methods that result from cloning a base member
695-
llvm::DenseSet<ValueDecl *> clonedMembers;
694+
// Map all cloned methods back to the original member
695+
llvm::DenseMap<ValueDecl *, ValueDecl *> clonedMembers;
696696

697697
public:
698698
llvm::DenseMap<const clang::ParmVarDecl*, FuncDecl*> defaultArgGenerators;
@@ -702,7 +702,7 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
702702
ValueDecl *importBaseMemberDecl(ValueDecl *decl, DeclContext *newContext,
703703
ClangInheritanceInfo inheritance);
704704

705-
bool isClonedMemberDecl(ValueDecl *decl);
705+
ValueDecl *getOriginalForClonedMember(const ValueDecl *decl);
706706

707707
static size_t getImportedBaseMemberDeclArity(const ValueDecl *valueDecl);
708708

test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,12 @@ struct IIOne : IOne {
1616
struct IIIOne : IIOne {
1717
int methodIII(void) const { return -111; }
1818
};
19+
20+
class Base {
21+
public:
22+
bool baseMethod() const { return true; }
23+
};
24+
25+
namespace Bar {
26+
class Derived : public Base {};
27+
} // namespace Bar

test/Interop/Cxx/class/inheritance/inherited-lookup-executable.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -cxx-interoperability-mode=default)
22
//
33
// REQUIRES: executable_test
4+
45
import InheritedLookup
56
import StdlibUnittest
67

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -cxx-interoperability-mode=default -enable-upcoming-feature MemberImportVisibility)
2+
//
3+
// REQUIRES: executable_test
4+
// REQUIRES: swift_feature_MemberImportVisibility
5+
6+
import InheritedLookup
7+
import StdlibUnittest
8+
9+
var InheritedMemberTestSuite = TestSuite("Test if inherited lookup works")
10+
11+
extension Bar.Derived {
12+
public func callBase() {
13+
let _ = baseMethod()
14+
}
15+
}
16+
17+
InheritedMemberTestSuite.test("Look up base methods from extensions") {
18+
let a = Bar.Derived()
19+
a.callBase()
20+
}
21+
22+
runAllTests()

0 commit comments

Comments
 (0)