Skip to content

Commit d237e2e

Browse files
authored
Merge pull request #83543 from susmonteiro/susmonteiro/overrides-of-pure-virtual-methods
[cxx-interop] Support overriding pure virtual methods from value types
2 parents 2ab459a + 5dd038f commit d237e2e

File tree

8 files changed

+343
-168
lines changed

8 files changed

+343
-168
lines changed

include/swift/AST/DiagnosticsClangImporter.def

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ GROUPED_WARNING(inconsistent_swift_name, ClangDeclarationImport, none,
7676
WARNING(swift_name_attr_ignored, none,
7777
"ignoring swift_name attribute %0; swift_name attributes have no "
7878
"effect on method overrides",
79-
(DeclName))
79+
(StringRef))
8080

8181
GROUPED_WARNING(swift_name_circular_context_import, ClangDeclarationImport, none,
8282
"cycle detected while resolving '%0' in swift_name attribute for '%1'",

lib/ClangImporter/ImportDecl.cpp

Lines changed: 5 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4353,42 +4353,11 @@ namespace {
43534353
// TODO: we don't have to import the actual `method` in this case,
43544354
// we can just synthesize a thunk and import that instead.
43554355

4356-
FuncDecl *result;
4357-
if (decl->size_overridden_methods() > 0) {
4358-
if (auto swiftNameAttr = decl->getAttr<clang::SwiftNameAttr>()) {
4359-
auto parsedDeclName = parseDeclName(swiftNameAttr->getName());
4360-
auto swiftDeclName =
4361-
parsedDeclName.formDeclName(method->getASTContext());
4362-
ImportedName importedName;
4363-
std::tie(importedName, std::ignore) = importFullName(decl);
4364-
4365-
result = synthesizer.makeVirtualMethod(decl);
4366-
4367-
if (swiftDeclName != importedName.getDeclName()) {
4368-
Impl.diagnose(HeaderLoc(swiftNameAttr->getLoc()),
4369-
diag::swift_name_attr_ignored, swiftDeclName);
4370-
4371-
Impl.markUnavailable(
4372-
result, (llvm::Twine("ignoring swift_name '") +
4373-
swiftNameAttr->getName() + "' in '" +
4374-
decl->getParent()->getName() +
4375-
"'; swift_name attributes have no effect "
4376-
"on method overrides")
4377-
.str());
4378-
}
4379-
} else {
4380-
// If there's no swift_name attribute, we don't import this method.
4381-
// This is because if the overridden method was renamed and
4382-
// this one is not, we want to use the overridden method's name.
4383-
// This is reasonable because `makeVirtualMethod` returns
4384-
// a thunk that will perform dynamic dispatch, and consequently
4385-
// the correct instance of the method will get executed.
4386-
return nullptr;
4387-
}
4388-
} else {
4389-
result = synthesizer.makeVirtualMethod(decl);
4390-
}
4391-
4356+
llvm::SmallString<64> swiftName;
4357+
funcDecl->getName().getString(swiftName);
4358+
FuncDecl *result =
4359+
synthesizer.makeVirtualMethod(decl, swiftName.str());
4360+
43924361
if (result) {
43934362
return result;
43944363
} else {

lib/ClangImporter/SwiftDeclSynthesizer.cpp

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "swift/AST/AttrKind.h"
1818
#include "swift/AST/Builtins.h"
1919
#include "swift/AST/Decl.h"
20+
#include "swift/AST/DiagnosticsClangImporter.h"
2021
#include "swift/AST/Expr.h"
2122
#include "swift/AST/ParameterList.h"
2223
#include "swift/AST/Pattern.h"
@@ -2285,7 +2286,7 @@ SwiftDeclSynthesizer::makeOperator(FuncDecl *operatorMethod,
22852286
// MARK: C++ virtual methods
22862287

22872288
FuncDecl *SwiftDeclSynthesizer::makeVirtualMethod(
2288-
const clang::CXXMethodDecl *clangMethodDecl) {
2289+
const clang::CXXMethodDecl *clangMethodDecl, StringRef swiftName) {
22892290
auto clangDC = clangMethodDecl->getParent();
22902291
auto &ctx = ImporterImpl.SwiftContext;
22912292

@@ -2296,6 +2297,25 @@ FuncDecl *SwiftDeclSynthesizer::makeVirtualMethod(
22962297
clangDC, clangDC, clangMethodDecl, ForwardingMethodKind::Virtual,
22972298
ReferenceReturnTypeBehaviorForBaseMethodSynthesis::KeepReference,
22982299
/*forceConstQualifier*/ false);
2300+
2301+
// If the override has a swift_name different from the base
2302+
// method, we ignore the swift_name attribute and instead use the base method's name.
2303+
// In this case, swiftName holds the correct derived method name obtained through NameImporter
2304+
if (clangMethodDecl->size_overridden_methods() > 0) {
2305+
if (auto oldSwiftNameAttr = newMethod->getAttr<clang::SwiftNameAttr>()) {
2306+
auto oldSwiftName = oldSwiftNameAttr->getName();
2307+
2308+
if (swiftName != oldSwiftName) {
2309+
ImporterImpl.diagnose(HeaderLoc(oldSwiftNameAttr->getLoc()),
2310+
diag::swift_name_attr_ignored,
2311+
oldSwiftName);
2312+
oldSwiftNameAttr->setName(newMethod->getASTContext(), swiftName);
2313+
}
2314+
} else {
2315+
newMethod->addAttr(clang::SwiftNameAttr::CreateImplicit(
2316+
newMethod->getASTContext(), swiftName));
2317+
}
2318+
}
22992319

23002320
auto result = dyn_cast_or_null<FuncDecl>(
23012321
ctx.getClangModuleLoader()->importDeclDirectly(newMethod));

lib/ClangImporter/SwiftDeclSynthesizer.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,8 @@ class SwiftDeclSynthesizer {
322322

323323
/// Given an overload of a C++ virtual method on a reference type, create a
324324
/// method that dispatches the call dynamically.
325-
FuncDecl *makeVirtualMethod(const clang::CXXMethodDecl *clangMethodDecl);
325+
FuncDecl *makeVirtualMethod(const clang::CXXMethodDecl *clangMethodDecl,
326+
StringRef swiftName);
326327

327328
FuncDecl *makeInstanceToStaticOperatorCallMethod(
328329
const clang::CXXMethodDecl *clangMethodDecl);

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

Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,8 @@ struct B2 : A1 {
181181

182182
int fooRename() const { return 222; }
183183

184-
__attribute__((swift_name("B2BarRename()"))) int barRename() const {
184+
__attribute__((swift_name("B2BarRename()")))
185+
int barRename() const {
185186
return 223;
186187
}
187188

@@ -274,3 +275,93 @@ struct D2 : B1, A2 {
274275
struct D3 : B1, B2 {};
275276

276277
struct D4 : C1, B2 {};
278+
279+
struct ValueType {
280+
virtual int virtualMethod() const { return 111; }
281+
282+
__attribute__((swift_name("swiftRenameMethodBase()")))
283+
virtual int renameMethodBase() const {
284+
return 112;
285+
}
286+
287+
virtual int renameMethodDerived() const { return 113; }
288+
289+
virtual int pureVirtualMethod() const = 0;
290+
291+
__attribute__((swift_name("swiftPureRenameBase()")))
292+
virtual int pureRenameBase() const = 0;
293+
294+
virtual int pureRenameDerived() const = 0;
295+
};
296+
297+
struct IMMORTAL_FRT DerivedFRTValueType : ValueType {
298+
virtual int virtualMethod() const override { return 211; }
299+
300+
virtual int renameMethodBase() const override { return 212; }
301+
302+
__attribute__((swift_name("swiftRenameMethodDerived()")))
303+
virtual int renameMethodDerived() const override {
304+
return 213;
305+
}
306+
307+
virtual int pureVirtualMethod() const override { return 214; }
308+
309+
virtual int pureRenameBase() const override { return 215; }
310+
311+
__attribute__((swift_name("swiftPureRenameDerived()")))
312+
virtual int pureRenameDerived() const override {
313+
return 216;
314+
}
315+
316+
static DerivedFRTValueType *_Nonnull create() {
317+
return new DerivedFRTValueType();
318+
}
319+
};
320+
321+
struct IMMORTAL_FRT EmptyDerivedFRTValueType : ValueType {};
322+
323+
struct DerivedValueType : ValueType {
324+
virtual int virtualMethod() const override { return 311; }
325+
326+
virtual int renameMethodBase() const override { return 312; }
327+
328+
__attribute__((swift_name("swiftRenameMethodDerived()"))) virtual int
329+
renameMethodDerived() const override {
330+
return 313;
331+
}
332+
333+
virtual int pureVirtualMethod() const override { return 314; }
334+
335+
virtual int pureRenameBase() const override { return 315; }
336+
337+
__attribute__((swift_name("swiftPureRenameDerived()"))) virtual int
338+
pureRenameDerived() const override {
339+
return 316;
340+
}
341+
};
342+
343+
struct IMMORTAL_FRT AbstractFRT {
344+
virtual int pureVirtualMethod() const = 0;
345+
346+
__attribute__((swift_name("swiftPureRenameBase()")))
347+
virtual int pureRenameBase() const = 0;
348+
349+
virtual int pureRenameDerived() const = 0;
350+
};
351+
352+
struct DerivedAbstractFRT : AbstractFRT {
353+
virtual int pureVirtualMethod() const override { return 211; }
354+
355+
virtual int pureRenameBase() const override { return 212; }
356+
357+
__attribute__((swift_name("swiftPureRenameDerived()")))
358+
virtual int pureRenameDerived() const override {
359+
return 213;
360+
}
361+
362+
static DerivedAbstractFRT *_Nonnull create() {
363+
return new DerivedAbstractFRT();
364+
}
365+
};
366+
367+
struct EmptyDerivedAbstractFRT : AbstractFRT {};

test/Interop/Cxx/foreign-reference/member-inheritance-module-interface.swift

Lines changed: 66 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -24,21 +24,13 @@
2424
// CHECK: }
2525

2626
// CHECK: class B1 {
27-
// CHECK: @available(*, unavailable, message: "ignoring{{.*}}")
28-
// CHECK: final func swiftVirtualMethod() -> Int32
29-
// CHECK: @available(*, unavailable, message: "ignoring{{.*}}")
30-
// CHECK: final func B1BarRename() -> Int32
31-
// CHECK: @available(*, unavailable, message: "ignoring{{.*}}")
32-
// CHECK: final func swiftParamsRename(b1 i: Int32) -> Int32
3327
// CHECK: final func virtualMethod() -> Int32
3428
// CHECK: final func swiftFooRename() -> Int32
3529
// CHECK: final func swiftBarRename() -> Int32
3630
// CHECK: final func swiftParamsRename(a1 i: Int32) -> Int32
3731
// CHECK: }
3832

3933
// CHECK: class B2 {
40-
// CHECK: @available(*, unavailable, message: "ignoring{{.*}}")
41-
// CHECK: final func B2BarRename() -> Int32
4234
// CHECK: final func virtualMethod() -> Int32
4335
// CHECK: final func swiftFooRename() -> Int32
4436
// CHECK: final func swiftBarRename() -> Int32
@@ -48,25 +40,11 @@
4840
// CHECK: class C1 {
4941
// CHECK: final func swiftFooRename() -> Int32
5042
// CHECK: final func swiftBarRename() -> Int32
51-
// CHECK: @available(*, unavailable, message: "ignoring{{.*}}")
52-
// CHECK: final func swiftVirtualMethod() -> Int32
53-
// CHECK: @available(*, unavailable, message: "ignoring{{.*}}")
54-
// CHECK: final func B1BarRename() -> Int32
55-
// CHECK: @available(*, unavailable, message: "ignoring{{.*}}")
56-
// CHECK: final func swiftParamsRename(b1 i: Int32) -> Int32
57-
// CHECK: final func virtualMethod() -> Int32
5843
// CHECK: final func swiftParamsRename(a1 i: Int32) -> Int32
44+
// CHECK: final func virtualMethod() -> Int32
5945
// CHECK: }
6046

6147
// CHECK: class C2 {
62-
// CHECK: @available(*, unavailable, message: "ignoring{{.*}}")
63-
// CHECK: final func swiftVirtualMethod() -> Int32
64-
// CHECK: @available(*, unavailable, message: "ignoring{{.*}}")
65-
// CHECK: final func C2FooRename() -> Int32
66-
// CHECK: @available(*, unavailable, message: "ignoring{{.*}}")
67-
// CHECK: final func B1BarRename() -> Int32
68-
// CHECK: @available(*, unavailable, message: "ignoring{{.*}}")
69-
// CHECK: final func swiftParamsRename(b1 i: Int32) -> Int32
7048
// CHECK: final func virtualMethod() -> Int32
7149
// CHECK: final func swiftFooRename() -> Int32
7250
// CHECK: final func swiftBarRename() -> Int32
@@ -91,32 +69,22 @@
9169
// CHECK: }
9270

9371
// CHECK: class D2 {
94-
// CHECK: @available(*, unavailable, message: "ignoring{{.*}}")
95-
// CHECK: final func swiftVirtualMethod() -> Int32
96-
// CHECK: @available(*, unavailable, message: "ignoring{{.*}}")
97-
// CHECK: final func B1BarRename() -> Int32
98-
// CHECK: @available(*, unavailable, message: "ignoring{{.*}}")
99-
// CHECK: final func swiftParamsRename(b1 i: Int32) -> Int32
72+
// CHECK: final func swiftFooRename() -> Int32
10073
// CHECK: @available(*, unavailable, message: "overrides{{.*}}")
10174
// CHECK: final func virtualMethod() -> Int32
102-
// CHECK: final func swiftFooRename() -> Int32
10375
// CHECK: @available(*, unavailable, message: "overrides{{.*}}")
10476
// CHECK: final func swiftBarRename() -> Int32
10577
// CHECK: @available(*, unavailable, message: "overrides{{.*}}")
10678
// CHECK: final func swiftParamsRename(a1 i: Int32) -> Int32
10779
// CHECK: @available(*, unavailable, message: "overrides{{.*}}")
80+
// CHECK: final func swiftVirtualMethod() -> Int32
81+
// CHECK: @available(*, unavailable, message: "overrides{{.*}}")
10882
// CHECK: final func A2BarRename() -> Int32
10983
// CHECK: @available(*, unavailable, message: "overrides{{.*}}")
11084
// CHECK: final func swiftParamsRename(a2 i: Int32) -> Int32
11185
// CHECK: }
11286

11387
// CHECK: struct D3 {
114-
// CHECK: @available(*, unavailable, message: "ignoring{{.*}}")
115-
// CHECK: final func swiftVirtualMethod() -> Int32
116-
// CHECK: @available(*, unavailable, message: "ignoring{{.*}}")
117-
// CHECK: final func B1BarRename() -> Int32
118-
// CHECK: @available(*, unavailable, message: "ignoring{{.*}}")
119-
// CHECK: final func swiftParamsRename(b1 i: Int32) -> Int32
12088
// CHECK: final func virtualMethod() -> Int32
12189
// CHECK: final func swiftFooRename() -> Int32
12290
// CHECK: final func swiftBarRename() -> Int32
@@ -126,12 +94,67 @@
12694
// CHECK: struct D4 {
12795
// CHECK: final func swiftFooRename() -> Int32
12896
// CHECK: final func swiftBarRename() -> Int32
129-
// CHECK: @available(*, unavailable, message: "ignoring{{.*}}")
130-
// CHECK: final func swiftVirtualMethod() -> Int32
131-
// CHECK: @available(*, unavailable, message: "ignoring{{.*}}")
132-
// CHECK: final func B1BarRename() -> Int32
133-
// CHECK: @available(*, unavailable, message: "ignoring{{.*}}")
134-
// CHECK: final func swiftParamsRename(b1 i: Int32) -> Int32
135-
// CHECK: final func virtualMethod() -> Int32
13697
// CHECK: final func swiftParamsRename(a1 i: Int32) -> Int32
98+
// CHECK: final func virtualMethod() -> Int32
99+
// CHECK: }
100+
101+
// CHECK: struct ValueType {
102+
// CHECK: func virtualMethod() -> Int32
103+
// CHECK: func swiftRenameMethodBase() -> Int32
104+
// CHECK: func renameMethodDerived() -> Int32
105+
// CHECK: @available(*, unavailable, message: "virtual function is not available in Swift because it is pure")
106+
// CHECK: func pureVirtualMethod() -> Int32
107+
// CHECK: @available(*, unavailable, message: "virtual function is not available in Swift because it is pure")
108+
// CHECK: func swiftPureRenameBase() -> Int32
109+
// CHECK: @available(*, unavailable, message: "virtual function is not available in Swift because it is pure")
110+
// CHECK: func pureRenameDerived() -> Int32
111+
// CHECK: }
112+
113+
// CHECK: class DerivedFRTValueType {
114+
// CHECK: final func virtualMethod() -> Int32
115+
// CHECK: final func swiftRenameMethodBase() -> Int32
116+
// CHECK: final func renameMethodDerived() -> Int32
117+
// CHECK: final func pureVirtualMethod() -> Int32
118+
// CHECK: final func swiftPureRenameBase() -> Int32
119+
// CHECK: final func pureRenameDerived() -> Int32
120+
// CHECK: }
121+
122+
// CHECK: class EmptyDerivedFRTValueType {
123+
// CHECK: func virtualMethod() -> Int32
124+
// CHECK: func swiftRenameMethodBase() -> Int32
125+
// CHECK: func renameMethodDerived() -> Int32
126+
// CHECK: @available(*, unavailable, message: "virtual function is not available in Swift because it is pure")
127+
// CHECK: func pureVirtualMethod() -> Int32
128+
// CHECK: @available(*, unavailable, message: "virtual function is not available in Swift because it is pure")
129+
// CHECK: func swiftPureRenameBase() -> Int32
130+
// CHECK: @available(*, unavailable, message: "virtual function is not available in Swift because it is pure")
131+
// CHECK: func pureRenameDerived() -> Int32
132+
// CHECK: }
133+
134+
// CHECK: struct DerivedValueType {
135+
// CHECK: func virtualMethod() -> Int32
136+
// CHECK: func swiftRenameMethodBase() -> Int32
137+
// CHECK: func renameMethodDerived() -> Int32
138+
// CHECK: func pureVirtualMethod() -> Int32
139+
// CHECK: func swiftPureRenameBase() -> Int32
140+
// CHECK: func pureRenameDerived() -> Int32
141+
// CHECK: }
142+
143+
// CHECK: class AbstractFRT {
144+
// CHECK: final func pureVirtualMethod() -> Int32
145+
// CHECK: final func swiftPureRenameBase() -> Int32
146+
// CHECK: final func pureRenameDerived() -> Int32
147+
// CHECK: }
148+
149+
// CHECK: class DerivedAbstractFRT {
150+
// CHECK: init()
151+
// CHECK: final func pureVirtualMethod() -> Int32
152+
// CHECK: final func swiftPureRenameBase() -> Int32
153+
// CHECK: final func pureRenameDerived() -> Int32
154+
// CHECK: }
155+
156+
// CHECK: class EmptyDerivedAbstractFRT {
157+
// CHECK: final func pureVirtualMethod() -> Int32
158+
// CHECK: final func swiftPureRenameBase() -> Int32
159+
// CHECK: final func pureRenameDerived() -> Int32
137160
// CHECK: }

0 commit comments

Comments
 (0)