Skip to content

Commit f72f574

Browse files
susmonteirosusmonteiro
committed
[cxx-interop] Adding swift_name attributes to virtual methods overrides
1 parent 21ded5c commit f72f574

File tree

9 files changed

+464
-6
lines changed

9 files changed

+464
-6
lines changed

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6336,11 +6336,9 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
63366336
// Capture the arity of already found members in the
63376337
// current record, to avoid adding ambiguous members
63386338
// from base classes.
6339-
const auto getArity =
6340-
ClangImporter::Implementation::getImportedBaseMemberDeclArity;
6341-
llvm::SmallSet<size_t, 4> foundNameArities;
6339+
llvm::SmallSet<DeclName, 4> foundMethodNames;
63426340
for (const auto *valueDecl : result)
6343-
foundNameArities.insert(getArity(valueDecl));
6341+
foundMethodNames.insert(valueDecl->getName());
63446342

63456343
for (auto base : cxxRecord->bases()) {
63466344
if (skipIfNonPublic && base.getAccessSpecifier() != clang::AS_public)
@@ -6376,7 +6374,7 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
63766374
for (auto foundInBase : baseResults) {
63776375
// Do not add duplicate entry with the same arity,
63786376
// as that would cause an ambiguous lookup.
6379-
if (foundNameArities.count(getArity(foundInBase)))
6377+
if (foundMethodNames.count(foundInBase->getName()))
63806378
continue;
63816379

63826380
collector.add(foundInBase);
@@ -7671,6 +7669,7 @@ ValueDecl *ClangImporter::Implementation::importBaseMemberDecl(
76717669
auto known = clonedBaseMembers.find(key);
76727670
if (known == clonedBaseMembers.end()) {
76737671
ValueDecl *cloned = cloneBaseMemberDecl(decl, newContext, inheritance);
7672+
handleUnavailableVirtualMethod(cloned);
76747673
known = clonedBaseMembers.insert({key, cloned}).first;
76757674
clonedMembers.insert(std::make_pair(cloned, decl));
76767675
}

‎lib/ClangImporter/ImportDecl.cpp

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
#include "swift/ClangImporter/ClangImporterRequests.h"
5656
#include "swift/ClangImporter/ClangModule.h"
5757
#include "swift/Parse/Lexer.h"
58+
#include "swift/Parse/ParseDeclName.h"
5859
#include "swift/Strings.h"
5960
#include "clang/AST/ASTContext.h"
6061
#include "clang/AST/Attr.h"
@@ -4424,7 +4425,43 @@ namespace {
44244425
// Create a thunk that will perform dynamic dispatch.
44254426
// TODO: we don't have to import the actual `method` in this case,
44264427
// we can just synthesize a thunk and import that instead.
4427-
auto result = synthesizer.makeVirtualMethod(decl);
4428+
4429+
FuncDecl *result;
4430+
if (decl->size_overridden_methods() > 0) {
4431+
if (auto swiftNameAttr = decl->getAttr<clang::SwiftNameAttr>()) {
4432+
auto parsedDeclName = parseDeclName(swiftNameAttr->getName());
4433+
auto swiftDeclName = parsedDeclName.formDeclName(method->getASTContext());
4434+
4435+
ImportedName importedName;
4436+
std::tie(importedName, std::ignore) = importFullName(decl);
4437+
4438+
if (swiftDeclName != importedName.getDeclName()) {
4439+
std::string unavailabilityMsg =
4440+
(llvm::Twine("ignoring swift_name '") +
4441+
swiftNameAttr->getName() + "' in '" +
4442+
decl->getParent()->getName() +
4443+
"'; swift_name attributes have no effect "
4444+
"in virtual methods overrides")
4445+
.str();
4446+
result = synthesizer.makeVirtualMethod(decl);
4447+
Impl.markUnavailable(result, unavailabilityMsg);
4448+
4449+
} else {
4450+
// If the swift_name attribute renames the function to the same as
4451+
// the overridden method, then we don't import this method.
4452+
// Since this is a thunk, it's enough to clone the base method
4453+
return nullptr;
4454+
}
4455+
} else {
4456+
// If there's no swift_name attribute, we don't import this
4457+
// method. This is because if the overridden method was renamed
4458+
// and this one is not, we want to use the overridden method's name
4459+
return nullptr;
4460+
}
4461+
} else {
4462+
result = synthesizer.makeVirtualMethod(decl);
4463+
}
4464+
44284465
if (result) {
44294466
return result;
44304467
} else {
@@ -10367,6 +10404,23 @@ ValueDecl *ClangImporter::Implementation::createUnavailableDecl(
1036710404
return var;
1036810405
}
1036910406

10407+
void ClangImporter::Implementation::handleUnavailableVirtualMethod(
10408+
ValueDecl *decl) {
10409+
if (!decl || decl->isUnavailable())
10410+
return;
10411+
10412+
auto *cxxRecordDecl = dyn_cast<clang::CXXRecordDecl>(
10413+
decl->getDeclContext()->getAsDecl()->getClangDecl());
10414+
10415+
if (!cxxRecordDecl)
10416+
return;
10417+
10418+
if (findUnavailableMethod(cxxRecordDecl, decl->getName())) {
10419+
markUnavailable(decl,
10420+
"overrides multiple C++ methods with different Swift names");
10421+
}
10422+
}
10423+
1037010424
// Force the members of the entire inheritance hierarchy to be loaded and
1037110425
// deserialized before loading the members of this class. This allows the
1037210426
// decl members table to be warmed up and enables the correct identification of

‎lib/ClangImporter/ImportName.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1706,6 +1706,12 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
17061706
}
17071707
}
17081708

1709+
// `swift_name` attribute is not supported in virtual methods overrides
1710+
if (auto method = dyn_cast<clang::CXXMethodDecl>(D)) {
1711+
if (method->isVirtual() && method->size_overridden_methods() > 0)
1712+
skipCustomName = true;
1713+
}
1714+
17091715
if (!skipCustomName) {
17101716
result.info.hasCustomName = true;
17111717
result.declName = parsedName.formDeclName(
@@ -2315,6 +2321,37 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
23152321
newName = baseName.substr(StringRef("__synthesizedVirtualCall_").size());
23162322
baseName = newName;
23172323
}
2324+
if (method->isVirtual()) {
2325+
// The name should be imported from the base method
2326+
if (method->size_overridden_methods() > 0) {
2327+
DeclName overriddenName;
2328+
bool foundDivergentMethod = false;
2329+
for (auto overriddenMethod : method->overridden_methods()) {
2330+
ImportedName importedName =
2331+
importName(overriddenMethod, version, givenName);
2332+
if (!overriddenName) {
2333+
overriddenName = importedName.getDeclName();
2334+
} else if (overriddenName.compare(importedName.getDeclName())) {
2335+
2336+
importerImpl->insertUnavailableMethod(method->getParent(),
2337+
importedName.getDeclName());
2338+
foundDivergentMethod = true;
2339+
}
2340+
}
2341+
2342+
if (foundDivergentMethod) {
2343+
importerImpl->insertUnavailableMethod(method->getParent(),
2344+
overriddenName);
2345+
return ImportedName();
2346+
}
2347+
baseName = overriddenName.getBaseIdentifier().str();
2348+
// Also inherit argument names from base method
2349+
argumentNames.clear();
2350+
for (auto argname : overriddenName.getArgumentNames()) {
2351+
argumentNames.push_back(argname.str());
2352+
}
2353+
}
2354+
}
23182355
}
23192356

23202357
// swift_newtype-ed declarations may have common words with the type name

‎lib/ClangImporter/ImporterImpl.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -694,6 +694,10 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
694694
// Map all cloned methods back to the original member
695695
llvm::DenseMap<ValueDecl *, ValueDecl *> clonedMembers;
696696

697+
// Keep track of methods that are unavailale in each class.
698+
llvm::DenseSet<std::pair<const clang::CXXRecordDecl *, DeclName>>
699+
unavailableMethods;
700+
697701
public:
698702
llvm::DenseMap<const clang::ParmVarDecl*, FuncDecl*> defaultArgGenerators;
699703

@@ -722,6 +726,18 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
722726
return getNameImporter().getEnumKind(decl);
723727
}
724728

729+
bool findUnavailableMethod(const clang::CXXRecordDecl *classDecl,
730+
DeclName name) {
731+
return unavailableMethods.contains({classDecl, name});
732+
}
733+
734+
void insertUnavailableMethod(const clang::CXXRecordDecl *classDecl,
735+
DeclName name) {
736+
unavailableMethods.insert({classDecl, name});
737+
}
738+
739+
void handleUnavailableVirtualMethod(ValueDecl *decl);
740+
725741
private:
726742
/// A mapping from imported declarations to their "alternate" declarations,
727743
/// for cases where a single Clang declaration is imported to two

‎lib/ClangImporter/SwiftLookupTable.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "swift/Basic/STLExtras.h"
2323
#include "swift/Basic/Version.h"
2424
#include "swift/ClangImporter/ClangImporter.h"
25+
#include "swift/Parse/ParseDeclName.h"
2526
#include "clang/AST/DeclCXX.h"
2627
#include "clang/AST/DeclObjC.h"
2728
#include "clang/Lex/MacroInfo.h"
@@ -1945,6 +1946,15 @@ void importer::addEntryToLookupTable(SwiftLookupTable &table,
19451946
named, importedName.getEffectiveContext());
19461947
}
19471948

1949+
if (auto swiftNameAttr = named->getAttr<clang::SwiftNameAttr>()) {
1950+
auto parsedDeclName = parseDeclName(swiftNameAttr->getName());
1951+
auto swiftDeclName =
1952+
parsedDeclName.formDeclName(nameImporter.getContext());
1953+
1954+
table.addEntry(swiftDeclName, named,
1955+
importedName.getEffectiveContext());
1956+
}
1957+
19481958
return true;
19491959
});
19501960
if (failed) {

‎test/Interop/Cxx/foreign-reference/Inputs/member-inheritance.h

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,3 +139,116 @@ inline const Immortal *_Nonnull castToImmortal(
139139
const DerivedFromImmortal *_Nonnull immortal) {
140140
return static_cast<const Immortal *>(immortal);
141141
}
142+
143+
// A1
144+
// |
145+
// B1
146+
// / \
147+
// C1 C2
148+
149+
struct IMMORTAL_FRT A1 {
150+
virtual int virtualMethod() const { return 111; }
151+
152+
__attribute__((swift_name("swiftFooRename()")))
153+
virtual int fooRename() const { return 112; }
154+
155+
__attribute__((swift_name("swiftBarRename()")))
156+
virtual int barRename() const { return 113; }
157+
158+
__attribute__((swift_name("swiftParamsRename(a1:)")))
159+
virtual int paramsRename(int i) const { return i; }
160+
};
161+
162+
struct B1 : A1 {
163+
__attribute__((swift_name("swiftVirtualMethod()")))
164+
virtual int virtualMethod() const override { return 211; }
165+
166+
virtual int fooRename() const override { return 212; }
167+
168+
__attribute__((swift_name("B1BarRename()")))
169+
virtual int barRename() const override { return 213; }
170+
171+
__attribute__((swift_name("swiftParamsRename(b1:)")))
172+
virtual int paramsRename(int i) const override { return i; }
173+
};
174+
175+
struct C1 : B1 {
176+
__attribute__((swift_name("swiftFooRename()")))
177+
virtual int fooRename() const override { return 312; }
178+
179+
__attribute__((swift_name("swiftBarRename()")))
180+
virtual int barRename() const override { return 313; }
181+
182+
virtual int paramsRename(int i) const override { return i; }
183+
};
184+
185+
struct C2 : B1 {
186+
__attribute__((swift_name("swiftVirtualMethod()")))
187+
virtual int virtualMethod() const override { return 321; }
188+
189+
__attribute__((swift_name("C2FooRename()")))
190+
virtual int fooRename() const override { return 322; }
191+
192+
__attribute__((swift_name("B1BarRename()")))
193+
virtual int barRename() const override { return 323; }
194+
195+
__attribute__((swift_name("swiftParamsRename(b1:)")))
196+
virtual int paramsRename(int i) const override { return i; }
197+
};
198+
199+
struct IMMORTAL_FRT A2 {
200+
__attribute__((swift_name("swiftVirtualMethod()")))
201+
virtual int virtualMethod() const { return 121; }
202+
203+
__attribute__((swift_name("swiftFooRename()")))
204+
virtual int fooRename() const { return 122; }
205+
206+
__attribute__((swift_name("A2BarRename()")))
207+
virtual int barRename() const { return 123; }
208+
209+
__attribute__((swift_name("swiftParamsRename(a2:)")))
210+
virtual int paramsRename(int i) const { return i; }
211+
};
212+
213+
// A1 A2
214+
// \ /
215+
// D1
216+
217+
struct D1 : A1, A2 {};
218+
219+
// A1 A2
220+
// \ /
221+
// B1 /
222+
// \ /
223+
// D2
224+
225+
struct D2 : B1, A2 {
226+
__attribute__((swift_name("swiftVirtualMethod()")))
227+
virtual int virtualMethod() const override { return 411; }
228+
229+
virtual int fooRename() const override { return 412; }
230+
231+
virtual int barRename() const override { return 413; }
232+
233+
virtual int paramsRename(int i) const override { return i; }
234+
};
235+
236+
// A1
237+
// / \
238+
// / \
239+
// B1 B2
240+
// |\ /|
241+
// | \ / |
242+
// | D3 |
243+
// C1 |
244+
// \ |
245+
// \ /
246+
// D4
247+
248+
struct B2 : A1 {
249+
virtual int virtualMethod() const override { return 221; }
250+
};
251+
252+
struct D3 : B1, B2 {};
253+
254+
struct D4 : C1, B2 {};

0 commit comments

Comments
 (0)