Skip to content

Commit cc8edf0

Browse files
beccadaxbeccadax
committed
Require @objc to be used with @implementation
…for extensions. This change also removes @implementation(CategoryName); you should attach the category name to the @objc attribute instead.
1 parent 6886108 commit cc8edf0

13 files changed

+210
-82
lines changed

‎include/swift/AST/Attr.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2422,6 +2422,9 @@ class DocumentationAttr: public DeclAttribute {
24222422

24232423
class ObjCImplementationAttr final : public DeclAttribute {
24242424
public:
2425+
/// Name of the category being implemented. This should only be used with
2426+
/// the early adopter \@\_objcImplementation syntax, but we support it there
2427+
/// for backwards compatibility.
24252428
Identifier CategoryName;
24262429

24272430
ObjCImplementationAttr(Identifier CategoryName, SourceLoc AtLoc,

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -763,6 +763,9 @@ ERROR(expr_selector_not_objc,none,
763763
NOTE(make_decl_objc,none,
764764
"add '@objc' to expose this %0 to Objective-C",
765765
(DescriptiveDeclKind))
766+
NOTE(make_decl_objc_for_implementation,none,
767+
"add '@objc' to implement an Objective-C %0",
768+
(DescriptiveDeclKind))
766769

767770
// Selectors-as-string-literals.
768771
WARNING(selector_literal_invalid,none,
@@ -1760,10 +1763,6 @@ WARNING(objc_implementation_early_spelling_deprecated,none,
17601763
ERROR(attr_objc_implementation_must_be_unconditional,none,
17611764
"only unconditional extensions can implement an Objective-C '@interface'",
17621765
())
1763-
ERROR(attr_objc_implementation_must_extend_class,none,
1764-
"cannot mark extension of %kind0 with '@_objcImplementation'; it is not "
1765-
"an imported Objective-C class",
1766-
(ValueDecl *))
17671766
ERROR(attr_objc_implementation_must_be_imported,none,
17681767
"'@_objcImplementation' cannot be used to extend %kind0 because it was "
17691768
"defined by a Swift 'class' declaration, not an imported Objective-C "
@@ -1800,6 +1799,14 @@ ERROR(attr_objc_implementation_raise_minimum_deployment_target,none,
18001799
"'@implementation' of an Objective-C class requires a minimum deployment "
18011800
"target of at least %0 %1",
18021801
(StringRef, llvm::VersionTuple))
1802+
ERROR(attr_implementation_requires_language,none,
1803+
"'@implementation' used without specifying the language being "
1804+
"implemented",
1805+
())
1806+
ERROR(attr_implementation_category_goes_on_objc_attr,none,
1807+
"Objective-C category should be specified on '@objc', not "
1808+
"'@implementation'",
1809+
())
18031810

18041811
ERROR(member_of_objc_implementation_not_objc_or_final,none,
18051812
"%kind0 does not match any %kindonly0 declared in the headers for %1; "
@@ -6295,7 +6302,7 @@ ERROR(objc_extension_not_class,none,
62956302
"'@objc' can only be applied to an extension of a class", ())
62966303

62976304
// If you change this, also change enum ObjCReason
6298-
#define OBJC_ATTR_SELECT "select{marked @_cdecl|marked dynamic|marked @objc|marked @objcMembers|marked @IBOutlet|marked @IBAction|marked @IBSegueAction|marked @NSManaged|a member of an @objc protocol|implicitly @objc|an @objc override|an implementation of an @objc requirement|marked @IBInspectable|marked @GKInspectable|in an @objc extension of a class (without @nonobjc)|in an @_objcImplementation extension of a class (without final or @nonobjc)|marked @objc by an access note}"
6305+
#define OBJC_ATTR_SELECT "select{marked @_cdecl|marked dynamic|marked @objc|marked @objcMembers|marked @IBOutlet|marked @IBAction|marked @IBSegueAction|marked @NSManaged|a member of an @objc protocol|implicitly @objc|an @objc override|an implementation of an @objc requirement|marked @IBInspectable|marked @GKInspectable|in an @objc extension of a class (without @nonobjc)|in an @objc @implementation extension of a class (without final or @nonobjc)|marked @objc by an access note}"
62996306

63006307
ERROR(objc_invalid_on_var,none,
63016308
"property cannot be %" OBJC_ATTR_SELECT "0 "

‎lib/AST/Decl.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3717,12 +3717,6 @@ Identifier ExtensionDecl::getObjCCategoryName() const {
37173717
return Identifier();
37183718
}
37193719

3720-
// Fall back to @_objcImplementation attribute.
3721-
if (auto implAttr =
3722-
getAttrs().getAttribute<ObjCImplementationAttr>(/*AllowInvalid=*/true)) {
3723-
return implAttr->CategoryName;
3724-
}
3725-
37263720
// Not a category, evidently.
37273721
return Identifier();
37283722
}

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6002,7 +6002,16 @@ constructResult(const llvm::TinyPtrVector<Decl *> &interfaces,
60026002
static bool isCategoryNameValid(ExtensionDecl *ext) {
60036003
auto attr = ext->getAttrs()
60046004
.getAttribute<ObjCImplementationAttr>(/*AllowInvalid=*/true);
6005-
return attr && !attr->isCategoryNameInvalid();
6005+
6006+
if (!attr)
6007+
return false;
6008+
6009+
// Clients using the stable syntax shouldn't have a category name on the attr.
6010+
// This is diagnosed in AttributeChecker::visitObjCImplementationAttr().
6011+
if (!attr->isEarlyAdopter() && !attr->CategoryName.empty())
6012+
return false;
6013+
6014+
return !attr->isCategoryNameInvalid();
60066015
}
60076016

60086017
static ObjCInterfaceAndImplementation

‎lib/Parse/ParseDecl.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7124,6 +7124,20 @@ Parser::parseDeclExtension(ParseDeclOptions Flags, DeclAttributes &Attributes) {
71247124
status |= whereStatus;
71257125
}
71267126

7127+
// @implementation requires an explicit @objc attribute, but
7128+
// @_objcImplementation didn't. Insert one if necessary.
7129+
auto implAttr = Attributes.getAttribute<ObjCImplementationAttr>();
7130+
if (implAttr && implAttr->isEarlyAdopter()
7131+
&& !Attributes.hasAttribute<ObjCAttr>()) {
7132+
ObjCAttr *objcAttr;
7133+
if (implAttr->CategoryName.empty())
7134+
objcAttr = ObjCAttr::createUnnamedImplicit(Context);
7135+
else
7136+
objcAttr = ObjCAttr::createNullary(Context, implAttr->CategoryName,
7137+
/*isNameImplicit=*/false);
7138+
Attributes.add(objcAttr);
7139+
}
7140+
71277141
ExtensionDecl *ext = ExtensionDecl::create(Context, ExtensionLoc,
71287142
extendedType.getPtrOrNull(),
71297143
Context.AllocateCopy(Inherited),

‎lib/Sema/TypeCheckAttr.cpp

Lines changed: 78 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1547,23 +1547,86 @@ static bool hasObjCImplementationFeature(Decl *D, ObjCImplementationAttr *attr,
15471547
return false;
15481548
}
15491549

1550+
static SmallString<64> makeObjCAttrInsertionText(Identifier categoryName) {
1551+
SmallString<64> objcAttrText;
1552+
objcAttrText += "@objc";
1553+
if (!categoryName.empty()) {
1554+
objcAttrText += "(";
1555+
objcAttrText += categoryName.str();
1556+
objcAttrText += ")";
1557+
}
1558+
objcAttrText += " ";
1559+
return objcAttrText;
1560+
}
1561+
1562+
static SourceRange getArgListRange(ASTContext &Ctx, DeclAttribute *attr) {
1563+
// attr->getRange() covers the attr name and argument list; adjust it to
1564+
// exclude the first token.
1565+
auto newStart = Lexer::getLocForEndOfToken(Ctx.SourceMgr,
1566+
attr->getRange().Start);
1567+
if (attr->getRange().contains(newStart)) {
1568+
return SourceRange(newStart, attr->getRange().End);
1569+
}
1570+
return SourceRange();
1571+
}
1572+
15501573
void AttributeChecker::
15511574
visitObjCImplementationAttr(ObjCImplementationAttr *attr) {
1575+
DeclAttribute * langAttr =
1576+
D->getAttrs().getAttribute<ObjCAttr>(/*AllowInvalid=*/true);
1577+
if (!langAttr)
1578+
langAttr = D->getAttrs().getAttribute<CDeclAttr>(/*AllowInvalid=*/true);
1579+
1580+
if (!langAttr) {
1581+
diagnose(attr->getLocation(), diag::attr_implementation_requires_language);
1582+
1583+
// If this is an extension, suggest '@objc'.
1584+
if (auto ED = dyn_cast<ExtensionDecl>(D)) {
1585+
SourceLoc insertLoc = ED->getAttrs().getStartLoc();
1586+
if (insertLoc.isInvalid())
1587+
insertLoc = ED->getStartLoc();
1588+
1589+
diagnose(attr->getLocation(), diag::make_decl_objc_for_implementation,
1590+
D->getDescriptiveKind())
1591+
.fixItInsert(insertLoc,
1592+
makeObjCAttrInsertionText(attr->CategoryName));
1593+
}
1594+
1595+
return;
1596+
}
1597+
15521598
if (auto ED = dyn_cast<ExtensionDecl>(D)) {
15531599
if (!hasObjCImplementationFeature(D, attr, Feature::ObjCImplementation))
15541600
return;
15551601

1602+
// Only early adopters should specify the category name on the attribute;
1603+
// the stabilized syntax uses @objc(CustomName) for that.
1604+
if (!attr->isEarlyAdopter() && !attr->CategoryName.empty()) {
1605+
auto diag = diagnose(attr->getLocation(),
1606+
diag::attr_implementation_category_goes_on_objc_attr);
1607+
1608+
auto argListRange = getArgListRange(Ctx, attr);
1609+
if (argListRange.isValid()) {
1610+
diag.fixItReplace(langAttr->getRangeWithAt(),
1611+
makeObjCAttrInsertionText(attr->CategoryName));
1612+
diag.fixItRemove(argListRange);
1613+
}
1614+
1615+
cast<ObjCAttr>(langAttr)->setName(ObjCSelector(Ctx, 0,
1616+
{attr->CategoryName}),
1617+
/*implicit=*/false);
1618+
1619+
return;
1620+
}
1621+
15561622
if (ED->isConstrainedExtension())
15571623
diagnoseAndRemoveAttr(attr,
15581624
diag::attr_objc_implementation_must_be_unconditional);
15591625

15601626
auto CD = dyn_cast<ClassDecl>(ED->getExtendedNominal());
15611627
if (!CD) {
1562-
diagnoseAndRemoveAttr(attr,
1563-
diag::attr_objc_implementation_must_extend_class,
1564-
ED->getExtendedNominal());
1565-
ED->getExtendedNominal()->diagnose(diag::decl_declared_here,
1566-
ED->getExtendedNominal());
1628+
// This will be diagnosed as diag::objc_extension_not_class.
1629+
attr->setInvalid();
15671630
return;
15681631
}
15691632

@@ -1590,14 +1653,14 @@ visitObjCImplementationAttr(ObjCImplementationAttr *attr) {
15901653
if (!attr->isCategoryNameInvalid() && !ED->getImplementedObjCDecl()) {
15911654
diagnose(attr->getLocation(),
15921655
diag::attr_objc_implementation_category_not_found,
1593-
attr->CategoryName, CD);
1594-
1595-
// attr->getRange() covers the attr name and argument list; adjust it to
1596-
// exclude the first token.
1597-
auto newStart = Lexer::getLocForEndOfToken(Ctx.SourceMgr,
1598-
attr->getRange().Start);
1599-
if (attr->getRange().contains(newStart)) {
1600-
auto argListRange = SourceRange(newStart, attr->getRange().End);
1656+
ED->getObjCCategoryName(), CD);
1657+
1658+
SourceRange argListRange;
1659+
if (attr->CategoryName.empty())
1660+
argListRange = getArgListRange(Ctx, langAttr);
1661+
else
1662+
argListRange = getArgListRange(Ctx, attr);
1663+
if (argListRange.isValid()) {
16011664
diagnose(attr->getLocation(),
16021665
diag::attr_objc_implementation_fixit_remove_category_name)
16031666
.fixItRemove(argListRange);
@@ -1628,12 +1691,8 @@ visitObjCImplementationAttr(ObjCImplementationAttr *attr) {
16281691
diagnose(attr->getLocation(),
16291692
diag::attr_objc_implementation_no_category_for_func, AFD);
16301693

1631-
// attr->getRange() covers the attr name and argument list; adjust it to
1632-
// exclude the first token.
1633-
auto newStart = Lexer::getLocForEndOfToken(Ctx.SourceMgr,
1634-
attr->getRange().Start);
1635-
if (attr->getRange().contains(newStart)) {
1636-
auto argListRange = SourceRange(newStart, attr->getRange().End);
1694+
auto argListRange = getArgListRange(Ctx, attr);
1695+
if (argListRange.isValid()) {
16371696
diagnostic.fixItRemove(argListRange);
16381697
}
16391698

‎lib/Sema/TypeCheckDecl.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -960,11 +960,6 @@ IsDynamicRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const {
960960
return true;
961961
}
962962

963-
// @_objcImplementation extension member implementations are implicitly
964-
// dynamic.
965-
if (decl->isObjCMemberImplementation())
966-
return true;
967-
968963
if (auto accessor = dyn_cast<AccessorDecl>(decl)) {
969964
// Runtime-replaceable accessors are dynamic when their storage declaration
970965
// is dynamic and they were explicitly defined or they are implicitly defined

‎lib/Sema/TypeCheckDeclObjC.cpp

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1249,6 +1249,11 @@ static bool isMemberOfObjCClassExtension(const ValueDecl *VD) {
12491249
return ext->getSelfClassDecl() && ext->getAttrs().hasAttribute<ObjCAttr>();
12501250
}
12511251

1252+
static bool isMemberOfObjCImplementationExtension(const ValueDecl *VD) {
1253+
return isMemberOfObjCClassExtension(VD) &&
1254+
cast<ExtensionDecl>(VD->getDeclContext())->isObjCImplementation();
1255+
}
1256+
12521257
/// Whether this declaration is a member of a class with the `@objcMembers`
12531258
/// attribute.
12541259
static bool isMemberOfObjCMembersClass(const ValueDecl *VD) {
@@ -1471,14 +1476,6 @@ static std::optional<ObjCReason> shouldMarkAsObjC(const ValueDecl *VD,
14711476
return ObjCReason(ObjCReason::MemberOfObjCProtocol);
14721477
}
14731478

1474-
// A member implementation of an @objcImplementation extension is @objc.
1475-
if (VD->isObjCMemberImplementation()) {
1476-
auto ext = VD->getDeclContext()->getAsDecl();
1477-
auto attr = ext->getAttrs()
1478-
.getAttribute<ObjCImplementationAttr>(/*AllowInvalid=*/true);
1479-
return ObjCReason(ObjCReason::MemberOfObjCImplementationExtension, attr);
1480-
}
1481-
14821479
// A @nonobjc is not @objc, even if it is an override of an @objc, so check
14831480
// for @nonobjc first.
14841481
if (VD->getAttrs().hasAttribute<NonObjCAttr>() ||
@@ -1487,10 +1484,23 @@ static std::optional<ObjCReason> shouldMarkAsObjC(const ValueDecl *VD,
14871484
.hasAttribute<NonObjCAttr>()))
14881485
return std::nullopt;
14891486

1490-
if (isMemberOfObjCClassExtension(VD) &&
1487+
if (isMemberOfObjCImplementationExtension(VD)) {
1488+
// A `final` member of an @objc @implementation extension is not @objc.
1489+
if (VD->isFinal())
1490+
return std::nullopt;
1491+
// Other members get a special reason.
1492+
if (canInferImplicitObjC(/*allowAnyAccess*/true)) {
1493+
auto ext = VD->getDeclContext()->getAsDecl();
1494+
auto attr = ext->getAttrs()
1495+
.getAttribute<ObjCImplementationAttr>(/*AllowInvalid=*/true);
1496+
return ObjCReason(ObjCReason::MemberOfObjCImplementationExtension, attr);
1497+
}
1498+
}
1499+
else if (isMemberOfObjCClassExtension(VD) &&
14911500
canInferImplicitObjC(/*allowAnyAccess*/true))
14921501
return ObjCReason(ObjCReason::MemberOfObjCExtension);
1493-
if (isMemberOfObjCMembersClass(VD) &&
1502+
1503+
if (isMemberOfObjCMembersClass(VD) &&
14941504
canInferImplicitObjC(/*allowAnyAccess*/false))
14951505
return ObjCReason(ObjCReason::MemberOfObjCMembersClass);
14961506

@@ -3802,6 +3812,18 @@ class ObjCImplementationChecker {
38023812
}
38033813
}
38043814

3815+
static SmallString<64> makeObjCAttrInsertionText(Identifier categoryName) {
3816+
SmallString<64> objcAttrText;
3817+
objcAttrText += "@objc";
3818+
if (!categoryName.empty()) {
3819+
objcAttrText += "(";
3820+
objcAttrText += categoryName.str();
3821+
objcAttrText += ")";
3822+
}
3823+
objcAttrText += " ";
3824+
return objcAttrText;
3825+
}
3826+
38053827
void diagnoseEarlyAdopterDeprecation() {
38063828
// Only encourage use of @implementation for early adopters, and only when
38073829
// there are no mismatches that they might be working around with it.
@@ -3822,9 +3844,12 @@ class ObjCImplementationChecker {
38223844
if (!isa<ExtensionDecl>(decl))
38233845
return;
38243846

3847+
auto attrText = makeObjCAttrInsertionText(getAttr()->CategoryName);
3848+
attrText += "@implementation";
3849+
38253850
diagnose(getAttr()->getLocation(),
38263851
diag::objc_implementation_early_spelling_deprecated)
3827-
.fixItReplace(getAttr()->getLocation(), "implementation");
3852+
.fixItReplace(getAttr()->getRangeWithAt(), attrText);
38283853
}
38293854
};
38303855
}

‎lib/Sema/TypeCheckObjC.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ class ObjCReason {
7171
ExplicitlyGKInspectable,
7272
/// Is it a member of an @objc extension of a class.
7373
MemberOfObjCExtension,
74-
/// Is it a member of an \@\_objcImplementation extension.
74+
/// Is it a member of an \@objc \@implementation extension.
7575
MemberOfObjCImplementationExtension,
7676
/// Has an explicit '@objc' attribute added by an access note, rather than
7777
/// written in source code.

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,15 @@
191191

192192
@end
193193

194+
@interface ObjCBadClass : NSObject
195+
@end
196+
197+
@interface ObjCBadClass (BadCategory1)
198+
@end
199+
200+
@interface ObjCBadClass (BadCategory2)
201+
@end
202+
194203
@protocol EmptyObjCProto
195204
@end
196205

0 commit comments

Comments
 (0)