Skip to content

Commit 9b24c83

Browse files
committed
[QoI] Don't emit @objc Fix-Its for cases where @objc will be inferred from a requirement.
This is a follow-up to the change that allowed one to omit @objc (or the name in an @objc) when it can be inferred by matching a requirement. There is no point in suggesting that one add @objc if it will be inferred anyway, since it's just syntactic noise.
1 parent 70f3290 commit 9b24c83

File tree

7 files changed

+64
-17
lines changed

7 files changed

+64
-17
lines changed

include/swift/AST/Decl.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2144,6 +2144,10 @@ class ValueDecl : public Decl {
21442144
/// first slot.
21452145
Optional<ObjCSelector> getObjCRuntimeName() const;
21462146

2147+
/// Determine whether the given declaration can infer @objc, or the
2148+
/// Objective-C name, if used to satisfy the given requirement.
2149+
bool canInferObjCFromRequirement(ValueDecl *requirement);
2150+
21472151
SourceLoc getNameLoc() const { return NameLoc; }
21482152
SourceLoc getLoc() const { return NameLoc; }
21492153

lib/AST/ASTContext.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2479,8 +2479,9 @@ bool ASTContext::diagnoseObjCUnsatisfiedOptReqConflicts(SourceFile &sf) {
24792479
fixDeclarationName(diag, conflicts[0], req->getFullName());
24802480

24812481
// Fix the '@objc' attribute, if needed.
2482-
fixDeclarationObjCName(diag, conflicts[0], req->getObjCRuntimeName(),
2483-
/*ignoreImpliedName=*/true);
2482+
if (!conflicts[0]->canInferObjCFromRequirement(req))
2483+
fixDeclarationObjCName(diag, conflicts[0], req->getObjCRuntimeName(),
2484+
/*ignoreImpliedName=*/true);
24842485
}
24852486

24862487
// @nonobjc will silence this warning.

lib/AST/Decl.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1736,6 +1736,42 @@ Optional<ObjCSelector> ValueDecl::getObjCRuntimeName() const {
17361736
return None;
17371737
}
17381738

1739+
bool ValueDecl::canInferObjCFromRequirement(ValueDecl *requirement) {
1740+
// Only makes sense for a requirement of an @objc protocol.
1741+
auto proto = cast<ProtocolDecl>(requirement->getDeclContext());
1742+
if (!proto->isObjC()) return false;
1743+
1744+
// Only makes sense when this declaration is within a nominal type
1745+
// or extension thereof.
1746+
auto nominal =
1747+
getDeclContext()->getAsNominalTypeOrNominalTypeExtensionContext();
1748+
if (!nominal) return false;
1749+
1750+
// If there is already an @objc attribute with an explicit name, we
1751+
// can't infer a name (it's already there).
1752+
if (auto objcAttr = getAttrs().getAttribute<ObjCAttr>()) {
1753+
if (!objcAttr->isNameImplicit()) return false;
1754+
}
1755+
1756+
// If the nominal type doesn't conform to the protocol at all, we
1757+
// cannot infer @objc no matter what we do.
1758+
SmallVector<ProtocolConformance *, 1> conformances;
1759+
if (!nominal->lookupConformance(getModuleContext(), proto, conformances))
1760+
return false;
1761+
1762+
// If any of the conformances is attributed to the context in which
1763+
// this declaration resides, we can infer @objc or the Objective-C
1764+
// name.
1765+
auto dc = getDeclContext();
1766+
for (auto conformance : conformances) {
1767+
if (conformance->getDeclContext() == dc)
1768+
return true;
1769+
}
1770+
1771+
// Nothing to infer from.
1772+
return false;
1773+
}
1774+
17391775
SourceLoc ValueDecl::getAttributeInsertionLoc(bool forModifier) const {
17401776
if (auto var = dyn_cast<VarDecl>(this)) {
17411777
if (auto pbd = var->getParentPatternBinding()) {

lib/Sema/TypeCheckProtocol.cpp

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1718,7 +1718,7 @@ diagnoseMatch(TypeChecker &tc, Module *module,
17181718
fixDeclarationName(diag, match.Witness, req->getFullName());
17191719

17201720
// Also fix the Objective-C name, if needed.
1721-
if (req->isObjC())
1721+
if (!match.Witness->canInferObjCFromRequirement(req))
17221722
fixDeclarationObjCName(diag, match.Witness, req->getObjCRuntimeName());
17231723
break;
17241724
}
@@ -3665,9 +3665,11 @@ void ConformanceChecker::checkConformance() {
36653665
: diag::witness_non_objc,
36663666
diagInfo.first, diagInfo.second,
36673667
Proto->getFullName());
3668-
fixDeclarationObjCName(
3669-
diag, witness,
3670-
cast<AbstractFunctionDecl>(requirement)->getObjCSelector());
3668+
if (!witness->canInferObjCFromRequirement(requirement)) {
3669+
fixDeclarationObjCName(
3670+
diag, witness,
3671+
cast<AbstractFunctionDecl>(requirement)->getObjCSelector());
3672+
}
36713673
} else if (isa<VarDecl>(witness)) {
36723674
auto diag = TC.diagnose(witness,
36733675
isOptional
@@ -3676,10 +3678,13 @@ void ConformanceChecker::checkConformance() {
36763678
/*isSubscript=*/false,
36773679
witness->getFullName(),
36783680
Proto->getFullName());
3679-
fixDeclarationObjCName(
3680-
diag, witness,
3681-
ObjCSelector(requirement->getASTContext(), 0,
3682-
cast<VarDecl>(requirement)->getObjCPropertyName()));
3681+
if (!witness->canInferObjCFromRequirement(requirement)) {
3682+
fixDeclarationObjCName(
3683+
diag, witness,
3684+
ObjCSelector(requirement->getASTContext(), 0,
3685+
cast<VarDecl>(requirement)
3686+
->getObjCPropertyName()));
3687+
}
36833688
} else if (isa<SubscriptDecl>(witness)) {
36843689
TC.diagnose(witness,
36853690
isOptional
@@ -4412,7 +4417,8 @@ static void diagnosePotentialWitness(TypeChecker &tc,
44124417
// Special case: note to add @objc.
44134418
auto diag = tc.diagnose(witness,
44144419
diag::optional_req_nonobjc_near_match_add_objc);
4415-
fixDeclarationObjCName(diag, witness, req->getObjCRuntimeName());
4420+
if (!witness->canInferObjCFromRequirement(req))
4421+
fixDeclarationObjCName(diag, witness, req->getObjCRuntimeName());
44164422
} else {
44174423
diagnoseMatch(tc, conformance->getDeclContext()->getParentModule(),
44184424
conformance, req, match);

test/ClangModules/objc_parse.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ func classAnyObject(_ obj: NSObject) {
265265
class Wobbler : NSWobbling { // expected-note{{candidate has non-matching type '()'}}
266266
@objc func wobble() { }
267267

268-
func returnMyself() -> Self { return self } // expected-error{{non-'@objc' method 'returnMyself()' does not satisfy requirement of '@objc' protocol 'NSWobbling'}}{{3-3=@objc }}
268+
func returnMyself() -> Self { return self } // expected-error{{non-'@objc' method 'returnMyself()' does not satisfy requirement of '@objc' protocol 'NSWobbling'}}{{none}}
269269
// expected-error@-1{{method cannot be an implementation of an @objc requirement because its result type cannot be represented in Objective-C}}
270270
}
271271

test/decl/protocol/conforms/near_miss_objc.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
class C1a : P1 {
1111
@objc func doSomething(a: Int, c: Double) { }
1212
// expected-warning@-1{{instance method 'doSomething(a:c:)' nearly matches optional requirement 'doSomething(a:b:)' of protocol 'P1'}}
13-
// expected-note@-2{{rename to 'doSomething(a:b:)' to satisfy this requirement}}{{34-34=b }}{{8-8=(doSomethingWithA:b:)}}
13+
// expected-note@-2{{rename to 'doSomething(a:b:)' to satisfy this requirement}}{{34-34=b }}{{none}}
1414
// expected-note@-3{{move 'doSomething(a:c:)' to an extension to silence this warning}}
1515
// expected-note@-4{{make 'doSomething(a:c:)' private to silence this warning}}{{9-9=private }}
1616
}
@@ -28,7 +28,7 @@ class C1c {
2828
extension C1c : P1 {
2929
func doSomething(a: Int, c: Double) { }
3030
// expected-warning@-1{{instance method 'doSomething(a:c:)' nearly matches optional requirement 'doSomething(a:b:)' of protocol 'P1'}}
31-
// expected-note@-2{{rename to 'doSomething(a:b:)' to satisfy this requirement}}{{28-28=b }}{{3-3=@objc(doSomethingWithA:b:) }}
31+
// expected-note@-2{{rename to 'doSomething(a:b:)' to satisfy this requirement}}{{28-28=b }}{{none}}
3232
// expected-note@-3{{move 'doSomething(a:c:)' to another extension to silence this warning}}
3333
// expected-note@-4{{make 'doSomething(a:c:)' private to silence this warning}}{{3-3=private }}
3434
}
@@ -87,7 +87,7 @@ class C4a : P4 {
8787
class C5a : P5 {
8888
func method(_: Int, for someClass: SomeClass, dividing double: Double) { }
8989
// expected-warning@-1{{instance method 'method(_:for:dividing:)' nearly matches optional requirement 'methodWithInt(_:forSomeClass:dividingDouble:)' of protocol 'P5'}}
90-
// expected-note@-2{{rename to 'methodWithInt(_:forSomeClass:dividingDouble:)' to satisfy this requirement}}{{3-3=@objc(methodWithInt:forSomeClass:dividingDouble:) }}{{8-14=methodWithInt}}{{23-26=forSomeClass}}{{49-57=dividingDouble}}
90+
// expected-note@-2{{rename to 'methodWithInt(_:forSomeClass:dividingDouble:)' to satisfy this requirement}}{{8-14=methodWithInt}}{{23-26=forSomeClass}}{{49-57=dividingDouble}}{{none}}
9191
// expected-note@-3{{move 'method(_:for:dividing:)' to an extension to silence this warning}}
9292
// expected-note@-4{{make 'method(_:for:dividing:)' private to silence this warning}}{{3-3=private }}
9393
}
@@ -100,7 +100,7 @@ class C5a : P5 {
100100
class C6a : P6 {
101101
func methodWithInt(_: Int, forSomeClass: SomeClass, dividingDouble: Double) { }
102102
// expected-warning@-1{{instance method 'methodWithInt(_:forSomeClass:dividingDouble:)' nearly matches optional requirement 'method(_:for:dividing:)' of protocol 'P6'}}
103-
// expected-note@-2{{rename to 'method(_:for:dividing:)' to satisfy this requirement}}{{3-3=@objc(method:for:dividing:) }}{{8-21=method}}{{30-30=for }}{{55-55=dividing }}
103+
// expected-note@-2{{rename to 'method(_:for:dividing:)' to satisfy this requirement}}{{8-21=method}}{{30-30=for }}{{55-55=dividing }}{{none}}
104104
// expected-note@-3{{move 'methodWithInt(_:forSomeClass:dividingDouble:)' to an extension to silence this warning}}
105105
// expected-note@-4{{make 'methodWithInt(_:forSomeClass:dividingDouble:)' private to silence this warning}}{{3-3=private }}
106106
}

test/decl/protocol/req/optional.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ extension C6 : P1 {
117117
@objc class C8 : P2 { // expected-note{{class 'C8' declares conformance to protocol 'P2' here}}
118118
func objcMethod(int x: Int) { } // expected-error{{Objective-C method 'objcMethodWithInt:' provided by method 'objcMethod(int:)' conflicts with optional requirement method 'method(y:)' in protocol 'P2'}}
119119
// expected-note@-1{{add '@nonobjc' to silence this error}}{{3-3=@nonobjc }}
120-
// expected-note@-2{{rename method to match requirement 'method(y:)'}}{{3-3=@objc(objcMethodWithInt:) }}{{8-18=method}}{{19-22=y}}
120+
// expected-note@-2{{rename method to match requirement 'method(y:)'}}{{8-18=method}}{{19-22=y}}{{none}}
121121
}
122122

123123

0 commit comments

Comments
 (0)