Skip to content

Commit 2217e5a

Browse files
beccadaxbeccadax
committed
Make @objcImpl work with @_cdecl
No real diagnostics yet, but we’re emitting mostly correct code.
1 parent 95eaeaf commit 2217e5a

File tree

15 files changed

+131
-59
lines changed

15 files changed

+131
-59
lines changed

‎include/swift/AST/Attr.def

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ DECL_ATTR(_restatedObjCConformance, RestatedObjCConformance,
246246
OnProtocol | UserInaccessible | LongAttribute | RejectByParser | NotSerialized | ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove,
247247
70)
248248
DECL_ATTR(_objcImplementation, ObjCImplementation,
249-
OnExtension | UserInaccessible | ABIBreakingToAdd | ABIBreakingToRemove | APIBreakingToAdd | APIBreakingToRemove,
249+
OnExtension | OnAbstractFunction | UserInaccessible | ABIBreakingToAdd | ABIBreakingToRemove | APIBreakingToAdd | APIBreakingToRemove,
250250
72)
251251
DECL_ATTR(_optimize, Optimize,
252252
OnAbstractFunction | OnSubscript | OnVar | UserInaccessible | ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove,

‎include/swift/AST/Decl.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -941,6 +941,11 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl> {
941941
/// attribute macro expansion.
942942
DeclAttributes getSemanticAttrs() const;
943943

944+
/// True if this declaration provides an implementation for an imported
945+
/// Objective-C declaration. This implies various restrictions and special
946+
/// behaviors for it and, if it's an extension, its members.
947+
bool isObjCImplementation() const;
948+
944949
using AuxiliaryDeclCallback = llvm::function_ref<void(Decl *)>;
945950

946951
/// Iterate over the auxiliary declarations for this declaration,
@@ -1836,11 +1841,6 @@ class ExtensionDecl final : public GenericContext, public Decl,
18361841
/// resiliently moved into the original protocol itself.
18371842
bool isEquivalentToExtendedContext() const;
18381843

1839-
/// True if this extension provides an implementation for an imported
1840-
/// Objective-C \c \@interface. This implies various restrictions and special
1841-
/// behaviors for its members.
1842-
bool isObjCImplementation() const;
1843-
18441844
/// Returns the name of the category specified by the \c \@_objcImplementation
18451845
/// attribute, or \c None if the name is invalid. Do not call unless
18461846
/// \c isObjCImplementation() returns \c true.

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1743,6 +1743,10 @@ ERROR(attr_objc_implementation_category_not_found,none,
17431743
NOTE(attr_objc_implementation_fixit_remove_category_name,none,
17441744
"remove arguments to implement the main '@interface' for this class",
17451745
())
1746+
ERROR(attr_objc_implementation_no_category_for_func,none,
1747+
"%kind0 does not belong to an Objective-C category; remove the category "
1748+
"name from this attribute",
1749+
(ValueDecl*))
17461750
ERROR(attr_objc_implementation_no_conformance,none,
17471751
"'@_objcImplementation' extension cannot add conformance to %0; "
17481752
"add this conformance %select{with an ordinary extension|"

‎lib/AST/Decl.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1847,7 +1847,7 @@ Type ExtensionDecl::getExtendedType() const {
18471847
return ErrorType::get(ctx);
18481848
}
18491849

1850-
bool ExtensionDecl::isObjCImplementation() const {
1850+
bool Decl::isObjCImplementation() const {
18511851
return getAttrs().hasAttribute<ObjCImplementationAttr>(/*AllowInvalid=*/true);
18521852
}
18531853

@@ -4404,7 +4404,8 @@ static bool checkAccess(const DeclContext *useDC, const ValueDecl *VD,
44044404
// a context where we would access its storage directly, forbid access. Name
44054405
// lookups will instead find and use the matching interface decl.
44064406
// FIXME: Passing `true` for `isAccessOnSelf` may cause false positives.
4407-
if (isObjCMemberImplementation(VD, getAccessLevel) &&
4407+
if ((VD->isObjCImplementation() ||
4408+
isObjCMemberImplementation(VD, getAccessLevel)) &&
44084409
VD->getAccessSemanticsFromContext(useDC, /*isAccessOnSelf=*/true)
44094410
!= AccessSemantics::DirectToStorage)
44104411
return false;

‎lib/IRGen/GenDecl.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3544,7 +3544,10 @@ llvm::Function *IRGenModule::getAddrOfSILFunction(
35443544
if (hasOrderNumber) {
35453545
auto &fnList = Module.getFunctionList();
35463546
fnList.remove(fn);
3547-
fnList.insert(llvm::Module::iterator(insertBefore), fn);
3547+
if (insertBefore)
3548+
fnList.insert(llvm::Module::iterator(insertBefore), fn);
3549+
else
3550+
fnList.push_back(fn);
35483551

35493552
EmittedFunctionsByOrder.insert(orderNumber, fn);
35503553
}

‎lib/PrintAsClang/DeclAndTypePrinter.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2858,12 +2858,16 @@ static bool hasExposeAttr(const ValueDecl *VD, bool isExtension = false) {
28582858
return false;
28592859
}
28602860

2861-
/// Skip \c \@objcImplementation \c extension member implementations and
2862-
/// overrides. They are already declared in handwritten headers, and they may
2863-
/// have attributes that aren't allowed in a category.
2861+
/// Skip \c \@objcImplementation functions, \c extension member
2862+
/// implementations, and overrides. They are already declared in handwritten
2863+
/// headers, and they may have attributes that aren't allowed in a category.
28642864
///
28652865
/// \return true if \p VD should \em not be included in the header.
28662866
static bool excludeForObjCImplementation(const ValueDecl *VD) {
2867+
// If it's an ObjC implementation (and not an extension, which might have
2868+
// members that need printing), skip it; it's declared elsewhere.
2869+
if (VD->isObjCImplementation() && ! isa<ExtensionDecl>(VD))
2870+
return true;
28672871
// Exclude member implementations; they are declared elsewhere.
28682872
if (VD->isObjCMemberImplementation())
28692873
return true;

‎lib/Sema/TypeCheckAttr.cpp

Lines changed: 64 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1517,60 +1517,79 @@ void AttributeChecker::visitNonObjCAttr(NonObjCAttr *attr) {
15171517

15181518
void AttributeChecker::
15191519
visitObjCImplementationAttr(ObjCImplementationAttr *attr) {
1520-
auto ED = cast<ExtensionDecl>(D);
1521-
if (ED->isConstrainedExtension())
1522-
diagnoseAndRemoveAttr(attr,
1523-
diag::attr_objc_implementation_must_be_unconditional);
1520+
if (auto ED = dyn_cast<ExtensionDecl>(D)) {
1521+
if (ED->isConstrainedExtension())
1522+
diagnoseAndRemoveAttr(attr,
1523+
diag::attr_objc_implementation_must_be_unconditional);
15241524

1525-
auto CD = dyn_cast<ClassDecl>(ED->getExtendedNominal());
1526-
if (!CD) {
1527-
diagnoseAndRemoveAttr(attr,
1528-
diag::attr_objc_implementation_must_extend_class,
1529-
ED->getExtendedNominal());
1530-
ED->getExtendedNominal()->diagnose(diag::decl_declared_here,
1531-
ED->getExtendedNominal());
1532-
return;
1533-
}
1525+
auto CD = dyn_cast<ClassDecl>(ED->getExtendedNominal());
1526+
if (!CD) {
1527+
diagnoseAndRemoveAttr(attr,
1528+
diag::attr_objc_implementation_must_extend_class,
1529+
ED->getExtendedNominal());
1530+
ED->getExtendedNominal()->diagnose(diag::decl_declared_here,
1531+
ED->getExtendedNominal());
1532+
return;
1533+
}
15341534

1535-
if (!CD->hasClangNode()) {
1536-
diagnoseAndRemoveAttr(attr, diag::attr_objc_implementation_must_be_imported,
1537-
CD);
1538-
CD->diagnose(diag::decl_declared_here, CD);
1539-
return;
1540-
}
1535+
if (!CD->hasClangNode()) {
1536+
diagnoseAndRemoveAttr(attr, diag::attr_objc_implementation_must_be_imported,
1537+
CD);
1538+
CD->diagnose(diag::decl_declared_here, CD);
1539+
return;
1540+
}
15411541

1542-
if (!CD->hasSuperclass()) {
1543-
diagnoseAndRemoveAttr(attr, diag::attr_objc_implementation_must_have_super,
1544-
CD);
1545-
CD->diagnose(diag::decl_declared_here, CD);
1546-
return;
1547-
}
1542+
if (!CD->hasSuperclass()) {
1543+
diagnoseAndRemoveAttr(attr, diag::attr_objc_implementation_must_have_super,
1544+
CD);
1545+
CD->diagnose(diag::decl_declared_here, CD);
1546+
return;
1547+
}
15481548

1549-
if (CD->isTypeErasedGenericClass()) {
1550-
diagnoseAndRemoveAttr(attr, diag::objc_implementation_cannot_have_generics,
1551-
CD);
1552-
CD->diagnose(diag::decl_declared_here, CD);
1553-
}
1549+
if (CD->isTypeErasedGenericClass()) {
1550+
diagnoseAndRemoveAttr(attr, diag::objc_implementation_cannot_have_generics,
1551+
CD);
1552+
CD->diagnose(diag::decl_declared_here, CD);
1553+
}
15541554

1555-
if (!attr->isCategoryNameInvalid() && !ED->getImplementedObjCDecl()) {
1556-
diagnose(attr->getLocation(),
1557-
diag::attr_objc_implementation_category_not_found,
1558-
attr->CategoryName, CD);
1559-
1560-
// attr->getRange() covers the attr name and argument list; adjust it to
1561-
// exclude the first token.
1562-
auto newStart = Lexer::getLocForEndOfToken(Ctx.SourceMgr,
1563-
attr->getRange().Start);
1564-
if (attr->getRange().contains(newStart)) {
1565-
auto argListRange = SourceRange(newStart, attr->getRange().End);
1555+
if (!attr->isCategoryNameInvalid() && !ED->getImplementedObjCDecl()) {
15661556
diagnose(attr->getLocation(),
1567-
diag::attr_objc_implementation_fixit_remove_category_name)
1568-
.fixItRemove(argListRange);
1557+
diag::attr_objc_implementation_category_not_found,
1558+
attr->CategoryName, CD);
1559+
1560+
// attr->getRange() covers the attr name and argument list; adjust it to
1561+
// exclude the first token.
1562+
auto newStart = Lexer::getLocForEndOfToken(Ctx.SourceMgr,
1563+
attr->getRange().Start);
1564+
if (attr->getRange().contains(newStart)) {
1565+
auto argListRange = SourceRange(newStart, attr->getRange().End);
1566+
diagnose(attr->getLocation(),
1567+
diag::attr_objc_implementation_fixit_remove_category_name)
1568+
.fixItRemove(argListRange);
1569+
}
1570+
1571+
attr->setCategoryNameInvalid();
1572+
1573+
return;
15691574
}
1575+
}
1576+
else if (auto AFD = dyn_cast<AbstractFunctionDecl>(D)) {
1577+
if (!attr->CategoryName.empty()) {
1578+
auto diagnostic =
1579+
diagnose(attr->getLocation(),
1580+
diag::attr_objc_implementation_no_category_for_func, AFD);
15701581

1571-
attr->setCategoryNameInvalid();
1582+
// attr->getRange() covers the attr name and argument list; adjust it to
1583+
// exclude the first token.
1584+
auto newStart = Lexer::getLocForEndOfToken(Ctx.SourceMgr,
1585+
attr->getRange().Start);
1586+
if (attr->getRange().contains(newStart)) {
1587+
auto argListRange = SourceRange(newStart, attr->getRange().End);
1588+
diagnostic.fixItRemove(argListRange);
1589+
}
15721590

1573-
return;
1591+
attr->setCategoryNameInvalid();
1592+
}
15741593
}
15751594
}
15761595

‎test/IRGen/Inputs/objc_implementation.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424

2525
@end
2626

27+
extern void implFunc(int param);
28+
2729

2830
@interface NoImplClass
2931

‎test/IRGen/objc_implementation.swift

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,13 @@ open class SwiftSubclass: ImplClass {
139139
// Functions
140140
//
141141

142+
@_objcImplementation @_cdecl("implFunc")
143+
public func implFunc(_ param: Int32) {}
144+
142145
public func fn(impl: ImplClass, swiftSub: SwiftSubclass) {
143146
impl.mainMethod(0)
144147
swiftSub.mainMethod(1)
148+
implFunc(2)
145149
}
146150

147151
// Swift calling convention -[ImplClass init]
@@ -239,12 +243,18 @@ public func fn(impl: ImplClass, swiftSub: SwiftSubclass) {
239243
// Swift calling convention SwiftSubclass.deinit (deallocating)
240244
// CHECK-LABEL: define swiftcc void @"$s19objc_implementation13SwiftSubclassCfD"
241245

246+
// inplFunc(_:)
247+
// CHECK-LABEL: define void @implFunc
248+
// FIXME: We'd like this to be internal or hidden, not public.
249+
// CHECK: define swiftcc void @"$s19objc_implementation8implFuncyys5Int32VF"
250+
242251
// fn(impl:swiftSub:)
243252
// CHECK-LABEL: define swiftcc void @"$s19objc_implementation2fn4impl8swiftSubySo9ImplClassC_AA13SwiftSubclassCtF"
244253
// CHECK: [[SEL_1:%[^ ]+]] = load ptr, ptr @"\01L_selector(mainMethod:)", align 8
245254
// CHECK: call void @objc_msgSend(ptr {{.*}}, ptr [[SEL_1]], i32 0)
246255
// CHECK: [[SEL_2:%[^ ]+]] = load ptr, ptr @"\01L_selector(mainMethod:)", align 8
247256
// CHECK: call void @objc_msgSend(ptr {{.*}}, ptr [[SEL_2]], i32 1)
257+
// CHECK: call void @implFunc
248258
// CHECK: ret void
249259
// CHECK: }
250260

‎test/Interpreter/Inputs/objc_implementation.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,6 @@ NS_ASSUME_NONNULL_BEGIN
1414

1515
@end
1616

17+
extern void implFunc(int param);
18+
1719
NS_ASSUME_NONNULL_END

‎test/Interpreter/objc_implementation.swift

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-run-simple-swift(-import-objc-header %S/Inputs/objc_implementation.h -D TOP_LEVEL_CODE -swift-version 5) %s | %FileCheck %s
1+
// RUN: %target-run-simple-swift(-import-objc-header %S/Inputs/objc_implementation.h -D TOP_LEVEL_CODE -swift-version 5) %s -save-temps | %FileCheck %s
22
// REQUIRES: executable_test
33
// REQUIRES: objc_interop
44

@@ -54,6 +54,8 @@ class LastWords {
5454
print("otherProperty =", swiftSub.otherProperty)
5555
print("implProperty =", swiftSub.implProperty)
5656
}
57+
58+
implFunc(2023 - 34)
5759
}
5860

5961
@objc func someMethod() -> String { "ImplClass.someMethod()" }
@@ -73,6 +75,10 @@ class SwiftSubclass: ImplClass {
7375
override func someMethod() -> String { "SwiftSubclass.someMethod()" }
7476
}
7577

78+
@_objcImplementation @_cdecl("implFunc") public func implFunc(_ param: Int32) {
79+
print("implFunc(\(param))")
80+
}
81+
7682
// `#if swift` to ignore the inactive branch's contents
7783
#if swift(>=5.0) && TOP_LEVEL_CODE
7884
ImplClass.runTests()
@@ -90,4 +96,5 @@ ImplClass.runTests()
9096
// CHECK: otherProperty = 13
9197
// CHECK: implProperty = 42
9298
// CHECK: SwiftSubclass It's better to burn out than to fade away.
99+
// CHECK: implFunc(1989)
93100
#endif

‎test/PrintAsObjC/Inputs/custom-modules/objc_implementation/objc_implementation.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,4 @@
66

77
@end
88

9+
void CImplFunc(void);

‎test/PrintAsObjC/objc_implementation.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,6 @@ extension ObjCClass {
3535
// NEGATIVE-NOT: )privateMethod{{ }}
3636
@objc private func privateMethod() -> Any? { nil }
3737
}
38+
39+
@_cdecl("CImplFunc") @_objcImplementation func CImplFunc() {}
40+
// NEGATIVE-NOT: CImplFunc(

‎test/decl/ext/Inputs/objc_implementation.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,8 @@
176176

177177
@end
178178

179-
179+
void CImplFunc1(int param);
180+
void CImplFunc2(int param);
180181

181182
struct ObjCStruct {
182183
int foo;

‎test/decl/ext/objc_implementation.swift

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,21 @@ protocol EmptySwiftProto {}
464464
// expected-error@-1 {{'@_objcImplementation' cannot be used to implement generic class 'ObjCImplGenericClass'}}
465465
}
466466

467+
@_objcImplementation @_cdecl("CImplFunc1")
468+
func CImplFunc1(_: Int32) {
469+
// OK
470+
}
471+
472+
@_objcImplementation(BadCategory) @_cdecl("CImplFunc2")
473+
func CImplFunc2(_: Int32) {
474+
// expected-error@-2 {{global function 'CImplFunc2' does not belong to an Objective-C category; remove the category name from this attribute}} {{21-34=}}
475+
}
476+
477+
@_objcImplementation @_cdecl("CImplFuncMissing")
478+
func CImplFuncMissing(_: Int32) {
479+
// FIXME: expected-DISABLED-error@-1 {{fnord}}
480+
}
481+
467482
func usesAreNotAmbiguous(obj: ObjCClass) {
468483
obj.method(fromHeader1: 1)
469484
obj.method(fromHeader2: 2)

0 commit comments

Comments
 (0)