Skip to content

Commit 84e4312

Browse files
beccadaxbeccadax
committed
Make @objcImpl work with @_cdecl
No real diagnostics yet, but we’re emitting mostly correct code.
1 parent b99a4f8 commit 84e4312

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
@@ -926,6 +926,11 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl> {
926926
/// attribute macro expansion.
927927
DeclAttributes getSemanticAttrs() const;
928928

929+
/// True if this declaration provides an implementation for an imported
930+
/// Objective-C declaration. This implies various restrictions and special
931+
/// behaviors for it and, if it's an extension, its members.
932+
bool isObjCImplementation() const;
933+
929934
using AuxiliaryDeclCallback = llvm::function_ref<void(Decl *)>;
930935

931936
/// Iterate over the auxiliary declarations for this declaration,
@@ -1813,11 +1818,6 @@ class ExtensionDecl final : public GenericContext, public Decl,
18131818
/// resiliently moved into the original protocol itself.
18141819
bool isEquivalentToExtendedContext() const;
18151820

1816-
/// True if this extension provides an implementation for an imported
1817-
/// Objective-C \c \@interface. This implies various restrictions and special
1818-
/// behaviors for its members.
1819-
bool isObjCImplementation() const;
1820-
18211821
/// Returns the name of the category specified by the \c \@_objcImplementation
18221822
/// attribute, or \c None if the name is invalid. Do not call unless
18231823
/// \c isObjCImplementation() returns \c true.

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1732,6 +1732,10 @@ ERROR(attr_objc_implementation_category_not_found,none,
17321732
NOTE(attr_objc_implementation_fixit_remove_category_name,none,
17331733
"remove arguments to implement the main '@interface' for this class",
17341734
())
1735+
ERROR(attr_objc_implementation_no_category_for_func,none,
1736+
"%kind0 does not belong to an Objective-C category; remove the category "
1737+
"name from this attribute",
1738+
(ValueDecl*))
17351739
ERROR(attr_objc_implementation_no_conformance,none,
17361740
"'@_objcImplementation' extension cannot add conformance to %0; "
17371741
"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
@@ -1840,7 +1840,7 @@ Type ExtensionDecl::getExtendedType() const {
18401840
return ErrorType::get(ctx);
18411841
}
18421842

1843-
bool ExtensionDecl::isObjCImplementation() const {
1843+
bool Decl::isObjCImplementation() const {
18441844
return getAttrs().hasAttribute<ObjCImplementationAttr>(/*AllowInvalid=*/true);
18451845
}
18461846

@@ -4356,7 +4356,8 @@ static bool checkAccess(const DeclContext *useDC, const ValueDecl *VD,
43564356
// a context where we would access its storage directly, forbid access. Name
43574357
// lookups will instead find and use the matching interface decl.
43584358
// FIXME: Passing `true` for `isAccessOnSelf` may cause false positives.
4359-
if (isObjCMemberImplementation(VD, getAccessLevel) &&
4359+
if ((VD->isObjCImplementation() ||
4360+
isObjCMemberImplementation(VD, getAccessLevel)) &&
43604361
VD->getAccessSemanticsFromContext(useDC, /*isAccessOnSelf=*/true)
43614362
!= AccessSemantics::DirectToStorage)
43624363
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
@@ -1481,60 +1481,79 @@ void AttributeChecker::visitNonObjCAttr(NonObjCAttr *attr) {
14811481

14821482
void AttributeChecker::
14831483
visitObjCImplementationAttr(ObjCImplementationAttr *attr) {
1484-
auto ED = cast<ExtensionDecl>(D);
1485-
if (ED->isConstrainedExtension())
1486-
diagnoseAndRemoveAttr(attr,
1487-
diag::attr_objc_implementation_must_be_unconditional);
1484+
if (auto ED = dyn_cast<ExtensionDecl>(D)) {
1485+
if (ED->isConstrainedExtension())
1486+
diagnoseAndRemoveAttr(attr,
1487+
diag::attr_objc_implementation_must_be_unconditional);
14881488

1489-
auto CD = dyn_cast<ClassDecl>(ED->getExtendedNominal());
1490-
if (!CD) {
1491-
diagnoseAndRemoveAttr(attr,
1492-
diag::attr_objc_implementation_must_extend_class,
1493-
ED->getExtendedNominal());
1494-
ED->getExtendedNominal()->diagnose(diag::decl_declared_here,
1495-
ED->getExtendedNominal());
1496-
return;
1497-
}
1489+
auto CD = dyn_cast<ClassDecl>(ED->getExtendedNominal());
1490+
if (!CD) {
1491+
diagnoseAndRemoveAttr(attr,
1492+
diag::attr_objc_implementation_must_extend_class,
1493+
ED->getExtendedNominal());
1494+
ED->getExtendedNominal()->diagnose(diag::decl_declared_here,
1495+
ED->getExtendedNominal());
1496+
return;
1497+
}
14981498

1499-
if (!CD->hasClangNode()) {
1500-
diagnoseAndRemoveAttr(attr, diag::attr_objc_implementation_must_be_imported,
1501-
CD);
1502-
CD->diagnose(diag::decl_declared_here, CD);
1503-
return;
1504-
}
1499+
if (!CD->hasClangNode()) {
1500+
diagnoseAndRemoveAttr(attr, diag::attr_objc_implementation_must_be_imported,
1501+
CD);
1502+
CD->diagnose(diag::decl_declared_here, CD);
1503+
return;
1504+
}
15051505

1506-
if (!CD->hasSuperclass()) {
1507-
diagnoseAndRemoveAttr(attr, diag::attr_objc_implementation_must_have_super,
1508-
CD);
1509-
CD->diagnose(diag::decl_declared_here, CD);
1510-
return;
1511-
}
1506+
if (!CD->hasSuperclass()) {
1507+
diagnoseAndRemoveAttr(attr, diag::attr_objc_implementation_must_have_super,
1508+
CD);
1509+
CD->diagnose(diag::decl_declared_here, CD);
1510+
return;
1511+
}
15121512

1513-
if (CD->isTypeErasedGenericClass()) {
1514-
diagnoseAndRemoveAttr(attr, diag::objc_implementation_cannot_have_generics,
1515-
CD);
1516-
CD->diagnose(diag::decl_declared_here, CD);
1517-
}
1513+
if (CD->isTypeErasedGenericClass()) {
1514+
diagnoseAndRemoveAttr(attr, diag::objc_implementation_cannot_have_generics,
1515+
CD);
1516+
CD->diagnose(diag::decl_declared_here, CD);
1517+
}
15181518

1519-
if (!attr->isCategoryNameInvalid() && !ED->getImplementedObjCDecl()) {
1520-
diagnose(attr->getLocation(),
1521-
diag::attr_objc_implementation_category_not_found,
1522-
attr->CategoryName, CD);
1523-
1524-
// attr->getRange() covers the attr name and argument list; adjust it to
1525-
// exclude the first token.
1526-
auto newStart = Lexer::getLocForEndOfToken(Ctx.SourceMgr,
1527-
attr->getRange().Start);
1528-
if (attr->getRange().contains(newStart)) {
1529-
auto argListRange = SourceRange(newStart, attr->getRange().End);
1519+
if (!attr->isCategoryNameInvalid() && !ED->getImplementedObjCDecl()) {
15301520
diagnose(attr->getLocation(),
1531-
diag::attr_objc_implementation_fixit_remove_category_name)
1532-
.fixItRemove(argListRange);
1521+
diag::attr_objc_implementation_category_not_found,
1522+
attr->CategoryName, CD);
1523+
1524+
// attr->getRange() covers the attr name and argument list; adjust it to
1525+
// exclude the first token.
1526+
auto newStart = Lexer::getLocForEndOfToken(Ctx.SourceMgr,
1527+
attr->getRange().Start);
1528+
if (attr->getRange().contains(newStart)) {
1529+
auto argListRange = SourceRange(newStart, attr->getRange().End);
1530+
diagnose(attr->getLocation(),
1531+
diag::attr_objc_implementation_fixit_remove_category_name)
1532+
.fixItRemove(argListRange);
1533+
}
1534+
1535+
attr->setCategoryNameInvalid();
1536+
1537+
return;
15331538
}
1539+
}
1540+
else if (auto AFD = dyn_cast<AbstractFunctionDecl>(D)) {
1541+
if (!attr->CategoryName.empty()) {
1542+
auto diagnostic =
1543+
diagnose(attr->getLocation(),
1544+
diag::attr_objc_implementation_no_category_for_func, AFD);
15341545

1535-
attr->setCategoryNameInvalid();
1546+
// attr->getRange() covers the attr name and argument list; adjust it to
1547+
// exclude the first token.
1548+
auto newStart = Lexer::getLocForEndOfToken(Ctx.SourceMgr,
1549+
attr->getRange().Start);
1550+
if (attr->getRange().contains(newStart)) {
1551+
auto argListRange = SourceRange(newStart, attr->getRange().End);
1552+
diagnostic.fixItRemove(argListRange);
1553+
}
15361554

1537-
return;
1555+
attr->setCategoryNameInvalid();
1556+
}
15381557
}
15391558
}
15401559

‎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)