Skip to content

Commit 3117e50

Browse files
xedinxedin
authored
Merge pull request #9376 from xedin/sr-4692
[QoI] Improve diagnostics for calling instance methods on type or in static context
2 parents 95e9681 + 5adeff0 commit 3117e50

File tree

4 files changed

+172
-81
lines changed

4 files changed

+172
-81
lines changed

‎lib/Sema/CSDiag.cpp

Lines changed: 114 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -2810,7 +2810,8 @@ diagnoseUnviableLookupResults(MemberLookupResult &result, Type baseObjTy,
28102810
}
28112811
assert(TypeDC->isTypeContext() && "Expected type decl context!");
28122812

2813-
if (TypeDC->getDeclaredTypeOfContext()->isEqual(instanceTy)) {
2813+
if (TypeDC->getAsNominalTypeOrNominalTypeExtensionContext() ==
2814+
instanceTy->getAnyNominal()) {
28142815
if (propertyInitializer)
28152816
CS->TC.diagnose(nameLoc, diag::instance_member_in_initializer,
28162817
memberName);
@@ -4931,6 +4932,115 @@ static bool diagnoseImplicitSelfErrors(Expr *fnExpr, Expr *argExpr,
49314932
return false;
49324933
}
49334934

4935+
// It is a somewhat common error to try to access an instance method as a
4936+
// curried member on the type, instead of using an instance, e.g. the user
4937+
// wrote:
4938+
//
4939+
// Foo.doThing(42, b: 19)
4940+
//
4941+
// instead of:
4942+
//
4943+
// myFoo.doThing(42, b: 19)
4944+
//
4945+
// Check for this situation and handle it gracefully.
4946+
static bool
4947+
diagnoseInstanceMethodAsCurriedMemberOnType(CalleeCandidateInfo &CCI,
4948+
Expr *fnExpr, Expr *argExpr) {
4949+
for (auto &candidate : CCI.candidates) {
4950+
auto argTy = candidate.getArgumentType();
4951+
if (!argTy)
4952+
return false;
4953+
4954+
auto *decl = candidate.getDecl();
4955+
if (!decl)
4956+
return false;
4957+
4958+
// If this is an exact match at the level 1 of the parameters, but
4959+
// there is still something wrong with the expression nevertheless
4960+
// it might be worth while to check if it's instance method as curried
4961+
// member of type problem.
4962+
if (CCI.closeness == CC_ExactMatch &&
4963+
(decl->isInstanceMember() && candidate.level == 1))
4964+
continue;
4965+
4966+
auto params = decomposeParamType(argTy, decl, candidate.level);
4967+
// If one of the candidates is an instance method with a single parameter
4968+
// at the level 0, this might be viable situation for calling instance
4969+
// method as curried member of type problem.
4970+
if (params.size() != 1 || !decl->isInstanceMember() || candidate.level > 0)
4971+
return false;
4972+
}
4973+
4974+
auto &TC = CCI.CS->TC;
4975+
4976+
if (auto UDE = dyn_cast<UnresolvedDotExpr>(fnExpr)) {
4977+
auto baseExpr = UDE->getBase();
4978+
auto baseType = baseExpr->getType();
4979+
if (auto *MT = baseType->getAs<MetatypeType>()) {
4980+
auto DC = CCI.CS->DC;
4981+
auto instanceType = MT->getInstanceType();
4982+
4983+
// If the base is an implicit self type reference, and we're in a
4984+
// an initializer, then the user wrote something like:
4985+
//
4986+
// class Foo { let val = initFn() }
4987+
// or
4988+
// class Bar { func something(x: Int = initFn()) }
4989+
//
4990+
// which runs in type context, not instance context. Produce a tailored
4991+
// diagnostic since this comes up and is otherwise non-obvious what is
4992+
// going on.
4993+
if (baseExpr->isImplicit() && isa<Initializer>(DC)) {
4994+
auto *TypeDC = DC->getParent();
4995+
bool propertyInitializer = true;
4996+
// If the parent context is not a type context, we expect it
4997+
// to be a defaulted parameter in a function declaration.
4998+
if (!TypeDC->isTypeContext()) {
4999+
assert(TypeDC->getContextKind() ==
5000+
DeclContextKind::AbstractFunctionDecl &&
5001+
"Expected function decl context for initializer!");
5002+
TypeDC = TypeDC->getParent();
5003+
propertyInitializer = false;
5004+
}
5005+
assert(TypeDC->isTypeContext() && "Expected type decl context!");
5006+
5007+
if (TypeDC->getAsNominalTypeOrNominalTypeExtensionContext() ==
5008+
instanceType->getAnyNominal()) {
5009+
if (propertyInitializer)
5010+
TC.diagnose(UDE->getLoc(), diag::instance_member_in_initializer,
5011+
UDE->getName());
5012+
else
5013+
TC.diagnose(UDE->getLoc(),
5014+
diag::instance_member_in_default_parameter,
5015+
UDE->getName());
5016+
return true;
5017+
}
5018+
}
5019+
5020+
// If this is a situation like this `self.foo(A())()` and self != A
5021+
// let's say that `self` is not convertible to A.
5022+
if (auto nominalType = argExpr->getType()->getAs<NominalType>()) {
5023+
if (!instanceType->isEqual(nominalType)) {
5024+
TC.diagnose(argExpr->getStartLoc(), diag::types_not_convertible,
5025+
false, nominalType, instanceType);
5026+
return true;
5027+
}
5028+
}
5029+
5030+
// Otherwise, complain about use of instance value on type.
5031+
auto diagnostic = isa<TypeExpr>(baseExpr)
5032+
? diag::instance_member_use_on_type
5033+
: diag::could_not_use_instance_member_on_type;
5034+
5035+
TC.diagnose(UDE->getLoc(), diagnostic, instanceType, UDE->getName())
5036+
.highlight(baseExpr->getSourceRange());
5037+
return true;
5038+
}
5039+
}
5040+
5041+
return false;
5042+
}
5043+
49345044
/// Emit a class of diagnostics that we only know how to generate when there is
49355045
/// exactly one candidate we know about. Return true if an error is emitted.
49365046
static bool diagnoseSingleCandidateFailures(CalleeCandidateInfo &CCI,
@@ -4949,69 +5059,6 @@ static bool diagnoseSingleCandidateFailures(CalleeCandidateInfo &CCI,
49495059
auto params = decomposeParamType(argTy, candidate.getDecl(), candidate.level);
49505060
auto args = decomposeArgType(argExpr->getType(), argLabels);
49515061

4952-
// It is a somewhat common error to try to access an instance method as a
4953-
// curried member on the type, instead of using an instance, e.g. the user
4954-
// wrote:
4955-
//
4956-
// Foo.doThing(42, b: 19)
4957-
//
4958-
// instead of:
4959-
//
4960-
// myFoo.doThing(42, b: 19)
4961-
//
4962-
// Check for this situation and handle it gracefully.
4963-
if (params.size() == 1 && candidate.getDecl() &&
4964-
candidate.getDecl()->isInstanceMember() &&
4965-
candidate.level == 0) {
4966-
if (auto UDE = dyn_cast<UnresolvedDotExpr>(fnExpr))
4967-
if (isa<TypeExpr>(UDE->getBase())) {
4968-
auto baseType = candidate.getArgumentType();
4969-
auto DC = CCI.CS->DC;
4970-
4971-
// If the base is an implicit self type reference, and we're in a
4972-
// an initializer, then the user wrote something like:
4973-
//
4974-
// class Foo { let val = initFn() }
4975-
// or
4976-
// class Bar { func something(x: Int = initFn()) }
4977-
//
4978-
// which runs in type context, not instance context. Produce a tailored
4979-
// diagnostic since this comes up and is otherwise non-obvious what is
4980-
// going on.
4981-
if (UDE->getBase()->isImplicit() && isa<Initializer>(DC)) {
4982-
auto *TypeDC = DC->getParent();
4983-
bool propertyInitializer = true;
4984-
// If the parent context is not a type context, we expect it
4985-
// to be a defaulted parameter in a function declaration.
4986-
if (!TypeDC->isTypeContext()) {
4987-
assert(TypeDC->getContextKind() ==
4988-
DeclContextKind::AbstractFunctionDecl &&
4989-
"Expected function decl context for initializer!");
4990-
TypeDC = TypeDC->getParent();
4991-
propertyInitializer = false;
4992-
}
4993-
assert(TypeDC->isTypeContext() && "Expected type decl context!");
4994-
4995-
if (TypeDC->getDeclaredTypeOfContext()->isEqual(baseType)) {
4996-
if (propertyInitializer)
4997-
TC.diagnose(UDE->getLoc(), diag::instance_member_in_initializer,
4998-
UDE->getName());
4999-
else
5000-
TC.diagnose(UDE->getLoc(),
5001-
diag::instance_member_in_default_parameter,
5002-
UDE->getName());
5003-
return true;
5004-
}
5005-
}
5006-
5007-
// Otherwise, complain about use of instance value on type.
5008-
TC.diagnose(UDE->getLoc(), diag::instance_member_use_on_type,
5009-
baseType, UDE->getName())
5010-
.highlight(UDE->getBase()->getSourceRange());
5011-
return true;
5012-
}
5013-
}
5014-
50155062
// Check the case where a raw-representable type is constructed from an
50165063
// argument with the same type:
50175064
//
@@ -5353,6 +5400,9 @@ bool FailureDiagnosis::diagnoseParameterErrors(CalleeCandidateInfo &CCI,
53535400
if (diagnoseImplicitSelfErrors(fnExpr, argExpr, CCI, argLabels, CS))
53545401
return true;
53555402

5403+
if (diagnoseInstanceMethodAsCurriedMemberOnType(CCI, fnExpr, argExpr))
5404+
return true;
5405+
53565406
// Do all the stuff that we only have implemented when there is a single
53575407
// candidate.
53585408
if (diagnoseSingleCandidateFailures(CCI, fnExpr, argExpr, argLabels))

‎test/ClangImporter/objc_parse.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -340,8 +340,8 @@ func testDynamicSelf(_ queen: Bee, wobbler: NSWobbling) {
340340
// Instance method on a base class with instancetype result, called on the
341341
// class itself.
342342
// FIXME: This should be accepted.
343-
// FIXME: The error is lousy, too.
344-
let baseClass: ObjCParseExtras.Base.Type = ObjCParseExtras.Base.returnMyself() // expected-error{{missing argument for parameter #1 in call}}
343+
let baseClass: ObjCParseExtras.Base.Type = ObjCParseExtras.Base.returnMyself()
344+
// expected-error@-1 {{instance member 'returnMyself' cannot be used on type 'Base'}}
345345
}
346346

347347
func testRepeatedProtocolAdoption(_ w: NSWindow) {

‎test/Constraints/diagnostics.swift

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -940,3 +940,29 @@ func ambiguousCall() -> Float {} // expected-note {{found this candidate}}
940940
func takesClosure(fn: () -> ()) {}
941941

942942
takesClosure() { ambiguousCall() } // expected-error {{ambiguous use of 'ambiguousCall()'}}
943+
944+
// SR-4692: Useless diagnostics calling non-static method
945+
946+
class SR_4692_a {
947+
private static func foo(x: Int, y: Bool) {
948+
self.bar(x: x)
949+
// expected-error@-1 {{instance member 'bar' cannot be used on type 'SR_4692_a'}}
950+
}
951+
952+
private func bar(x: Int) {
953+
}
954+
}
955+
956+
class SR_4692_b {
957+
static func a() {
958+
self.f(x: 3, y: true)
959+
// expected-error@-1 {{instance member 'f' cannot be used on type 'SR_4692_b'}}
960+
}
961+
962+
private func f(a: Int, b: Bool, c: String) {
963+
self.f(x: a, y: b)
964+
}
965+
966+
private func f(x: Int, y: Bool) {
967+
}
968+
}

‎test/NameBinding/name_lookup.swift

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ class ThisBase1 {
1212
set {}
1313
}
1414

15-
func baseFunc0() {} // expected-note 2 {{'baseFunc0()' declared here}}
15+
func baseFunc0() {}
1616
func baseFunc1(_ a: Int) {}
1717

1818
subscript(i: Int) -> Double {
@@ -58,7 +58,7 @@ class ThisDerived1 : ThisBase1 {
5858
set {}
5959
}
6060

61-
func derivedFunc0() {} // expected-note {{'derivedFunc0()' declared here}}
61+
func derivedFunc0() {}
6262
func derivedFunc1(_ a: Int) {}
6363

6464
subscript(i: Double) -> Int {
@@ -222,20 +222,20 @@ class ThisDerived1 : ThisBase1 {
222222
class func staticTestSelf1() {
223223
self.baseInstanceVar = 42 // expected-error {{member 'baseInstanceVar' cannot be used on type 'ThisDerived1'}}
224224
self.baseProp = 42 // expected-error {{member 'baseProp' cannot be used on type 'ThisDerived1'}}
225-
self.baseFunc0() // expected-error {{missing argument}}
226-
self.baseFunc0(ThisBase1())() // expected-error {{'(ThisBase1) -> () -> ()' is not convertible to '(ThisDerived1) -> () -> ()'}}
225+
self.baseFunc0() // expected-error {{instance member 'baseFunc0' cannot be used on type 'ThisDerived1'}}
226+
self.baseFunc0(ThisBase1())() // expected-error {{'ThisBase1' is not convertible to 'ThisDerived1'}}
227227

228228
self.baseFunc0(ThisDerived1())()
229-
self.baseFunc1(42) // expected-error {{cannot convert value of type 'Int' to expected argument type 'ThisBase1'}}
230-
self.baseFunc1(ThisBase1())(42) // expected-error {{'(ThisBase1) -> (Int) -> ()' is not convertible to '(ThisDerived1) -> (Int) -> ()'}}
229+
self.baseFunc1(42) // expected-error {{instance member 'baseFunc1' cannot be used on type 'ThisDerived1'}}
230+
self.baseFunc1(ThisBase1())(42) // expected-error {{'ThisBase1' is not convertible to 'ThisDerived1'}}
231231
self.baseFunc1(ThisDerived1())(42)
232232
self[0] = 42.0 // expected-error {{instance member 'subscript' cannot be used on type 'ThisDerived1'}}
233233
self.baseStaticVar = 42
234234
self.baseStaticProp = 42
235235
self.baseStaticFunc0()
236236

237237
self.baseExtProp = 42 // expected-error {{member 'baseExtProp' cannot be used on type 'ThisDerived1'}}
238-
self.baseExtFunc0() // expected-error {{missing argument}}
238+
self.baseExtFunc0() // expected-error {{instance member 'baseExtFunc0' cannot be used on type 'ThisDerived1'}}
239239
self.baseExtStaticVar = 42
240240
self.baseExtStaticProp = 42 // expected-error {{member 'baseExtStaticProp' cannot be used on type 'ThisDerived1'}}
241241
self.baseExtStaticFunc0()
@@ -252,15 +252,15 @@ class ThisDerived1 : ThisBase1 {
252252

253253
self.derivedInstanceVar = 42 // expected-error {{member 'derivedInstanceVar' cannot be used on type 'ThisDerived1'}}
254254
self.derivedProp = 42 // expected-error {{member 'derivedProp' cannot be used on type 'ThisDerived1'}}
255-
self.derivedFunc0() // expected-error {{missing argument}}
256-
self.derivedFunc0(ThisBase1())() // expected-error {{cannot convert value of type 'ThisBase1' to expected argument type 'ThisDerived1'}}
255+
self.derivedFunc0() // expected-error {{instance member 'derivedFunc0' cannot be used on type 'ThisDerived1'}}
256+
self.derivedFunc0(ThisBase1())() // expected-error {{'ThisBase1' is not convertible to 'ThisDerived1'}}
257257
self.derivedFunc0(ThisDerived1())()
258258
self.derivedStaticVar = 42
259259
self.derivedStaticProp = 42
260260
self.derivedStaticFunc0()
261261

262262
self.derivedExtProp = 42 // expected-error {{member 'derivedExtProp' cannot be used on type 'ThisDerived1'}}
263-
self.derivedExtFunc0() // expected-error {{missing argument}}
263+
self.derivedExtFunc0() // expected-error {{instance member 'derivedExtFunc0' cannot be used on type 'ThisDerived1'}}
264264
self.derivedExtStaticVar = 42
265265
self.derivedExtStaticProp = 42 // expected-error {{member 'derivedExtStaticProp' cannot be used on type 'ThisDerived1'}}
266266
self.derivedExtStaticFunc0()
@@ -291,17 +291,17 @@ class ThisDerived1 : ThisBase1 {
291291
class func staticTestSuper1() {
292292
super.baseInstanceVar = 42 // expected-error {{member 'baseInstanceVar' cannot be used on type 'ThisBase1'}}
293293
super.baseProp = 42 // expected-error {{member 'baseProp' cannot be used on type 'ThisBase1'}}
294-
super.baseFunc0() // expected-error {{missing argument}}
294+
super.baseFunc0() // expected-error {{instance member 'baseFunc0' cannot be used on type 'ThisBase1'}}
295295
super.baseFunc0(ThisBase1())()
296-
super.baseFunc1(42) // expected-error {{cannot convert value of type 'Int' to expected argument type 'ThisBase1'}}
296+
super.baseFunc1(42) // expected-error {{instance member 'baseFunc1' cannot be used on type 'ThisBase1'}}
297297
super.baseFunc1(ThisBase1())(42)
298298
super[0] = 42.0 // expected-error {{instance member 'subscript' cannot be used on type 'ThisBase1'}}
299299
super.baseStaticVar = 42
300300
super.baseStaticProp = 42
301301
super.baseStaticFunc0()
302302

303303
super.baseExtProp = 42 // expected-error {{member 'baseExtProp' cannot be used on type 'ThisBase1'}}
304-
super.baseExtFunc0() // expected-error {{missing argument}}
304+
super.baseExtFunc0() // expected-error {{instance member 'baseExtFunc0' cannot be used on type 'ThisBase1'}}
305305
super.baseExtStaticVar = 42
306306
super.baseExtStaticProp = 42 // expected-error {{member 'baseExtStaticProp' cannot be used on type 'ThisBase1'}}
307307
super.baseExtStaticFunc0()
@@ -349,7 +349,7 @@ extension ThisBase1 {
349349
set {}
350350
}
351351

352-
func baseExtFunc0() {} // expected-note 2 {{'baseExtFunc0()' declared here}}
352+
func baseExtFunc0() {}
353353

354354
var baseExtStaticVar: Int // expected-error {{extensions may not contain stored properties}} // expected-note 2 {{did you mean 'baseExtStaticVar'?}}
355355

@@ -381,7 +381,7 @@ extension ThisDerived1 {
381381
set {}
382382
}
383383

384-
func derivedExtFunc0() {} // expected-note {{'derivedExtFunc0()' declared here}}
384+
func derivedExtFunc0() {}
385385

386386
var derivedExtStaticVar: Int // expected-error {{extensions may not contain stored properties}}
387387

@@ -485,6 +485,12 @@ class Test19935319 {
485485
func getFoo() -> Int {}
486486
}
487487

488+
class Test19935319G<T> {
489+
let i = getFoo()
490+
// expected-error@-1 {{cannot use instance member 'getFoo' within property initializer; property initializers run before 'self' is available}}
491+
func getFoo() -> Int { return 42 }
492+
}
493+
488494
// <rdar://problem/27013358> Crash using instance member as default parameter
489495
class rdar27013358 {
490496
let defaultValue = 1
@@ -495,6 +501,15 @@ class rdar27013358 {
495501
init(another value: Int = returnTwo()) {} // expected-error {{cannot use instance member 'returnTwo' as a default parameter}}
496502
}
497503

504+
class rdar27013358G<T> {
505+
let defaultValue = 1
506+
func returnTwo() -> Int {
507+
return 2
508+
}
509+
init(defaulted value: Int = defaultValue) {} // expected-error {{cannot use instance member 'defaultValue' as a default parameter}}
510+
init(another value: Int = returnTwo()) {} // expected-error {{cannot use instance member 'returnTwo' as a default parameter}}
511+
}
512+
498513
// <rdar://problem/23904262> QoI: ivar default initializer cannot reference other default initialized ivars?
499514
class r23904262 {
500515
let x = 1

0 commit comments

Comments
 (0)