Skip to content

Commit 4d59d71

Browse files
committed
[Conformance checking] Apply omit-needless-words for near-miss checking.
The differences between Swift 2 and Swift 3 names can be very significant, when the Swift 2 names have a lot of restated type information. These differences end up disabling the near-miss heuristics because the magnitude of the change is so high. Therefore, apply the omit-needless-words heuristics to the potential witness and the requirement before scoring them. Should finish up rdar://problem/25159872 for real.
1 parent d817f3c commit 4d59d71

File tree

4 files changed

+78
-19
lines changed

4 files changed

+78
-19
lines changed

lib/Sema/MiscDiagnostics.cpp

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2532,13 +2532,10 @@ static OmissionTypeName getTypeNameForOmission(Type type) {
25322532
return "";
25332533
}
25342534

2535-
/// Attempt to omit needless words from the name of the given declaration.
2536-
static Optional<DeclName> omitNeedlessWords(AbstractFunctionDecl *afd) {
2535+
Optional<DeclName> swift::omitNeedlessWords(AbstractFunctionDecl *afd) {
25372536
auto &Context = afd->getASTContext();
2538-
if (!Context.LangOpts.WarnOmitNeedlessWords)
2539-
return None;
25402537

2541-
if (afd->isInvalid())
2538+
if (afd->isInvalid() || isa<DestructorDecl>(afd))
25422539
return None;
25432540

25442541
DeclName name = afd->getFullName();
@@ -2624,16 +2621,12 @@ static Optional<DeclName> omitNeedlessWords(AbstractFunctionDecl *afd) {
26242621
return DeclName(Context, newBaseName, newArgNames);
26252622
}
26262623

2627-
/// Attempt to omit needless words from the name of the given declaration.
2628-
static Optional<Identifier> omitNeedlessWords(VarDecl *var) {
2624+
Optional<Identifier> swift::omitNeedlessWords(VarDecl *var) {
26292625
auto &Context = var->getASTContext();
26302626

26312627
if (var->isInvalid())
26322628
return None;
26332629

2634-
if (!Context.LangOpts.WarnOmitNeedlessWords)
2635-
return None;
2636-
26372630
if (var->getName().empty())
26382631
return None;
26392632

@@ -2646,7 +2639,7 @@ static Optional<Identifier> omitNeedlessWords(VarDecl *var) {
26462639

26472640
// Dig out the type of the variable.
26482641
Type type = var->getInterfaceType()->getReferenceStorageReferent()
2649-
->getLValueOrInOutObjectType();
2642+
->getLValueOrInOutObjectType();
26502643
while (auto optObjectTy = type->getAnyOptionalObjectType())
26512644
type = optObjectTy;
26522645

@@ -2674,7 +2667,10 @@ static Optional<Identifier> omitNeedlessWords(VarDecl *var) {
26742667
}
26752668

26762669
void TypeChecker::checkOmitNeedlessWords(AbstractFunctionDecl *afd) {
2677-
auto newName = ::omitNeedlessWords(afd);
2670+
if (!Context.LangOpts.WarnOmitNeedlessWords)
2671+
return;
2672+
2673+
auto newName = omitNeedlessWords(afd);
26782674
if (!newName)
26792675
return;
26802676

@@ -2685,7 +2681,10 @@ void TypeChecker::checkOmitNeedlessWords(AbstractFunctionDecl *afd) {
26852681
}
26862682

26872683
void TypeChecker::checkOmitNeedlessWords(VarDecl *var) {
2688-
auto newName = ::omitNeedlessWords(var);
2684+
if (!Context.LangOpts.WarnOmitNeedlessWords)
2685+
return;
2686+
2687+
auto newName = omitNeedlessWords(var);
26892688
if (!newName)
26902689
return;
26912690

@@ -2859,7 +2858,7 @@ void TypeChecker::checkOmitNeedlessWords(ApplyExpr *apply) {
28592858
return;
28602859

28612860
// Determine whether the callee has any needless words in it.
2862-
auto newName = ::omitNeedlessWords(afd);
2861+
auto newName = omitNeedlessWords(afd);
28632862

28642863
bool renamed;
28652864
if (!newName) {
@@ -2961,7 +2960,7 @@ void TypeChecker::checkOmitNeedlessWords(MemberRefExpr *memberRef) {
29612960
return;
29622961

29632962
// Check whether any needless words were omitted.
2964-
auto newName = ::omitNeedlessWords(var);
2963+
auto newName = omitNeedlessWords(var);
29652964
if (!newName)
29662965
return;
29672966

lib/Sema/TypeCheckProtocol.cpp

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4188,6 +4188,34 @@ static unsigned scorePotentiallyMatchingNames(DeclName lhs, DeclName rhs,
41884188
return score;
41894189
}
41904190

4191+
/// Apply omit-needless-words to the given declaration, if possible.
4192+
static Optional<DeclName> omitNeedlessWords(ValueDecl *value) {
4193+
if (auto func = dyn_cast<AbstractFunctionDecl>(value))
4194+
return swift::omitNeedlessWords(func);
4195+
if (auto var = dyn_cast<VarDecl>(value)) {
4196+
if (auto newName = swift::omitNeedlessWords(var))
4197+
return DeclName(*newName);
4198+
return None;
4199+
}
4200+
return None;
4201+
}
4202+
4203+
/// Determine the score between two potentially-matching declarations.
4204+
static unsigned scorePotentiallyMatching(ValueDecl *req, ValueDecl *witness,
4205+
unsigned limit) {
4206+
DeclName reqName = req->getFullName();
4207+
DeclName witnessName = witness->getFullName();
4208+
4209+
// Apply the omit-needless-words heuristics to both names.
4210+
if (auto adjustedReqName = ::omitNeedlessWords(req))
4211+
reqName = *adjustedReqName;
4212+
if (auto adjustedWitnessName = ::omitNeedlessWords(witness))
4213+
witnessName = *adjustedWitnessName;
4214+
4215+
return scorePotentiallyMatchingNames(reqName, witnessName, isa<FuncDecl>(req),
4216+
limit);
4217+
}
4218+
41914219
namespace {
41924220
/// Describes actions one could take to suppress a warning about a
41934221
/// nearly-matching witness for an optional requirement.
@@ -4499,10 +4527,8 @@ void TypeChecker::checkConformancesInContext(DeclContext *dc,
44994527
unsigned bestScore = UINT_MAX;
45004528
for (auto req : unsatisfiedReqs) {
45014529
// Score this particular optional requirement.
4502-
auto score = scorePotentiallyMatchingNames(value->getFullName(),
4503-
req->getFullName(),
4504-
isa<FuncDecl>(req),
4505-
bestScore);
4530+
auto score = scorePotentiallyMatching(req, value, bestScore);
4531+
45064532
// If the score is better than the best we've seen, update the best
45074533
// and clear out the list.
45084534
if (score < bestScore) {

lib/Sema/TypeChecker.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1848,6 +1848,12 @@ class EncodedDiagnosticMessage {
18481848
const StringRef Message;
18491849
};
18501850

1851+
/// Attempt to omit needless words from the name of the given declaration.
1852+
Optional<DeclName> omitNeedlessWords(AbstractFunctionDecl *afd);
1853+
1854+
/// Attempt to omit needless words from the name of the given declaration.
1855+
Optional<Identifier> omitNeedlessWords(VarDecl *var);
1856+
18511857
} // end namespace swift
18521858

18531859
#endif

test/decl/protocol/conforms/near_miss_objc.swift

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,31 @@ class C4a : P4 {
7676
// expected-note@-3{{move 'doSomethingWithPriority(_:d:)' to an extension to silence this warning}}
7777
// expected-note@-4{{make 'doSomethingWithPriority(_:d:)' private to silence this warning}}{{3-3=private }}
7878
}
79+
80+
@objc class SomeClass { }
81+
82+
@objc protocol P5 {
83+
optional func methodWithInt(_: Int, forSomeClass: SomeClass, dividingDouble: Double)
84+
// expected-note@-1{{requirement 'methodWithInt(_:forSomeClass:dividingDouble:)' declared here}}
85+
}
86+
87+
class C5a : P5 {
88+
func method(_: Int, for someClass: SomeClass, dividing double: Double) { }
89+
// 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}}
91+
// expected-note@-3{{move 'method(_:for:dividing:)' to an extension to silence this warning}}
92+
// expected-note@-4{{make 'method(_:for:dividing:)' private to silence this warning}}{{3-3=private }}
93+
}
94+
95+
@objc protocol P6 {
96+
optional func method(_: Int, for someClass: SomeClass, dividing double: Double)
97+
// expected-note@-1{{requirement 'method(_:for:dividing:)' declared here}}
98+
}
99+
100+
class C6a : P6 {
101+
func methodWithInt(_: Int, forSomeClass: SomeClass, dividingDouble: Double) { }
102+
// 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 }}
104+
// expected-note@-3{{move 'methodWithInt(_:forSomeClass:dividingDouble:)' to an extension to silence this warning}}
105+
// expected-note@-4{{make 'methodWithInt(_:forSomeClass:dividingDouble:)' private to silence this warning}}{{3-3=private }}
106+
}

0 commit comments

Comments
 (0)