Skip to content

Commit 22aa032

Browse files
slavapestovslavapestov
committed
Sema: Clean up SE-0036 diagnostic a bit
Suggest a fix-it for unqualified references to all static members from instance context, not just enum elements. Also, fix a small problem with the fix-it for replacing protocol names with 'Self' inside extension bodies -- we didn't handle nested functions properly.
1 parent 3ebd53d commit 22aa032

File tree

6 files changed

+98
-68
lines changed

6 files changed

+98
-68
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ ERROR(could_not_use_type_member_on_instance,none,
9696
ERROR(could_not_use_enum_element_on_instance,none,
9797
"enum element %0 cannot be referenced as an instance member",
9898
(DeclName))
99-
ERROR(could_not_use_type_member_on_existential,none,
99+
ERROR(could_not_use_type_member_on_protocol_metatype,none,
100100
"static member %1 cannot be used on protocol metatype %0",
101101
(Type, DeclName))
102102
ERROR(could_not_use_instance_member_on_type,none,

‎lib/Sema/CSDiag.cpp

Lines changed: 76 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1948,10 +1948,13 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
19481948
/// the exact expression kind).
19491949
bool diagnoseGeneralMemberFailure(Constraint *constraint);
19501950

1951-
/// Diagnose the lookup of an enum element as instance member where only a
1952-
/// static member is allowed
1953-
void diagnoseEnumInstanceMemberLookup(EnumElementDecl *enumElementDecl,
1954-
SourceLoc loc);
1951+
/// Diagnose the lookup of an static member or enum element as instance member.
1952+
void diagnoseTypeMemberOnInstanceLookup(Type baseObjTy,
1953+
Expr *baseExpr,
1954+
DeclName memberName,
1955+
DeclNameLoc nameLoc,
1956+
ValueDecl *member,
1957+
SourceLoc loc);
19551958

19561959
/// Given a result of name lookup that had no viable results, diagnose the
19571960
/// unviable ones.
@@ -2338,12 +2341,54 @@ bool FailureDiagnosis::diagnoseGeneralMemberFailure(Constraint *constraint) {
23382341
}
23392342

23402343
void FailureDiagnosis::
2341-
diagnoseEnumInstanceMemberLookup(EnumElementDecl *enumElementDecl,
2342-
SourceLoc loc) {
2343-
auto diag = diagnose(loc, diag::could_not_use_enum_element_on_instance,
2344-
enumElementDecl->getName());
2345-
auto parentEnum = enumElementDecl->getParentEnum();
2346-
auto enumMetatype = parentEnum->getType()->castTo<AnyMetatypeType>();
2344+
diagnoseTypeMemberOnInstanceLookup(Type baseObjTy,
2345+
Expr *baseExpr,
2346+
DeclName memberName,
2347+
DeclNameLoc nameLoc,
2348+
ValueDecl *member,
2349+
SourceLoc loc) {
2350+
SourceRange baseRange = baseExpr ? baseExpr->getSourceRange() : SourceRange();
2351+
2352+
// If the base of the lookup is a protocol metatype, suggest
2353+
// to replace the metatype with 'Self'
2354+
// error saying the lookup cannot be on a protocol metatype
2355+
if (auto metatypeTy = baseObjTy->getAs<MetatypeType>()) {
2356+
auto Diag = diagnose(loc,
2357+
diag::could_not_use_type_member_on_protocol_metatype,
2358+
baseObjTy, memberName);
2359+
Diag.highlight(baseRange).highlight(nameLoc.getSourceRange());
2360+
2361+
// See through function decl context
2362+
if (auto parent = CS->DC->getInnermostTypeContext()) {
2363+
// If we are in a protocol extension of 'Proto' and we see
2364+
// 'Proto.static', suggest 'Self.static'
2365+
if (auto extensionContext = parent->getAsProtocolExtensionContext()) {
2366+
if (extensionContext->getDeclaredType()->getCanonicalType()
2367+
== metatypeTy->getInstanceType()->getCanonicalType()) {
2368+
Diag.fixItReplace(baseRange, "Self");
2369+
}
2370+
}
2371+
}
2372+
2373+
return;
2374+
}
2375+
2376+
// Otherwise the static member lookup was invalid because it was
2377+
// called on an instance
2378+
Optional<InFlightDiagnostic> Diag;
2379+
2380+
if (isa<EnumElementDecl>(member))
2381+
Diag.emplace(diagnose(loc, diag::could_not_use_enum_element_on_instance,
2382+
memberName));
2383+
else
2384+
Diag.emplace(diagnose(loc, diag::could_not_use_type_member_on_instance,
2385+
baseObjTy, memberName));
2386+
2387+
Diag->highlight(nameLoc.getSourceRange());
2388+
2389+
// No fix-it if the lookup was qualified
2390+
if (baseExpr && !baseExpr->isImplicit())
2391+
return;
23472392

23482393
// Determine the contextual type of the expression
23492394
Type contextualType;
@@ -2355,8 +2400,8 @@ diagnoseEnumInstanceMemberLookup(EnumElementDecl *enumElementDecl,
23552400

23562401
// Try to provide a fix-it that only contains a '.'
23572402
if (contextualType) {
2358-
if (enumMetatype->getInstanceType()->isEqual(contextualType)) {
2359-
diag.fixItInsert(loc, ".");
2403+
if (baseObjTy->isEqual(contextualType)) {
2404+
Diag->fixItInsert(loc, ".");
23602405
return;
23612406
}
23622407
}
@@ -2382,8 +2427,8 @@ diagnoseEnumInstanceMemberLookup(EnumElementDecl *enumElementDecl,
23822427
// If the rhs of '~=' is the enum type, a single dot suffixes
23832428
// since the type can be inferred
23842429
Type secondArgType = binaryExpr->getArg()->getElement(1)->getType();
2385-
if (secondArgType->isEqual(enumMetatype->getInstanceType())) {
2386-
diag.fixItInsert(loc, ".");
2430+
if (secondArgType->isEqual(baseObjTy)) {
2431+
Diag->fixItInsert(loc, ".");
23872432
return;
23882433
}
23892434
}
@@ -2392,12 +2437,14 @@ diagnoseEnumInstanceMemberLookup(EnumElementDecl *enumElementDecl,
23922437
}
23932438

23942439
// Fall back to a fix-it with a full type qualifier
2395-
SmallString<32> enumTypeName;
2396-
llvm::raw_svector_ostream typeNameStream(enumTypeName);
2397-
typeNameStream << parentEnum->getName();
2398-
typeNameStream << ".";
2399-
2400-
diag.fixItInsert(loc, typeNameStream.str());
2440+
auto nominal =
2441+
member->getDeclContext()
2442+
->getAsNominalTypeOrNominalTypeExtensionContext();
2443+
SmallString<32> typeName;
2444+
llvm::raw_svector_ostream typeNameStream(typeName);
2445+
typeNameStream << nominal->getName() << ".";
2446+
2447+
Diag->fixItInsert(loc, typeNameStream.str());
24012448
return;
24022449
}
24032450

@@ -2461,8 +2508,12 @@ diagnoseUnviableLookupResults(MemberLookupResult &result, Type baseObjTy,
24612508
// because there is exactly one candidate!) diagnose this.
24622509
bool sameProblem = true;
24632510
auto firstProblem = result.UnviableCandidates[0].second;
2464-
for (auto cand : result.UnviableCandidates)
2511+
ValueDecl *member = nullptr;
2512+
for (auto cand : result.UnviableCandidates) {
2513+
if (member == nullptr)
2514+
member = cand.first;
24652515
sameProblem &= cand.second == firstProblem;
2516+
}
24662517

24672518
auto instanceTy = baseObjTy;
24682519
if (auto *MTT = instanceTy->getAs<AnyMetatypeType>())
@@ -2527,48 +2578,11 @@ diagnoseUnviableLookupResults(MemberLookupResult &result, Type baseObjTy,
25272578
instanceTy, memberName)
25282579
.highlight(baseRange).highlight(nameLoc.getSourceRange());
25292580
return;
2530-
2531-
case MemberLookupResult::UR_TypeMemberOnInstance:
2532-
if (instanceTy->isExistentialType() && baseObjTy->is<AnyMetatypeType>()) {
2533-
// If the base of the lookup is an existential metatype, emit an
2534-
// error saying the lookup cannot be on a protocol metatype
2535-
auto Diag = diagnose(loc, diag::could_not_use_type_member_on_existential,
2536-
baseObjTy, memberName);
2537-
Diag.highlight(baseRange).highlight(nameLoc.getSourceRange());
2538-
2539-
// See through function decl context
2540-
if (auto parent = CS->DC->getParent())
2541-
// If we are in a protocol extension of 'Proto' and we see
2542-
// 'Proto.static', suggest 'Self.static'
2543-
if (auto extensionContext = parent->getAsProtocolExtensionContext()) {
2544-
if (extensionContext->getDeclaredType()->getCanonicalType()
2545-
== instanceTy->getCanonicalType()) {
2546-
Diag.fixItReplace(baseRange, "Self");
2547-
}
2548-
}
25492581

2550-
} else {
2551-
// Otherwise the static member lookup was invalid because it was
2552-
// called on an instance
2553-
2554-
// Handle enum element lookup on instance type
2555-
auto lookThroughBaseObjTy = baseObjTy->lookThroughAllAnyOptionalTypes();
2556-
if (lookThroughBaseObjTy->is<EnumType>()
2557-
|| lookThroughBaseObjTy->is<BoundGenericEnumType>()) {
2558-
for (auto cand : result.UnviableCandidates) {
2559-
ValueDecl *decl = cand.first;
2560-
if (auto enumElementDecl = dyn_cast<EnumElementDecl>(decl)) {
2561-
diagnoseEnumInstanceMemberLookup(enumElementDecl, loc);
2562-
return;
2563-
}
2564-
}
2565-
}
2566-
2567-
// Provide diagnostic other static member lookups on instance type
2568-
diagnose(loc, diag::could_not_use_type_member_on_instance,
2569-
baseObjTy, memberName)
2570-
.highlight(baseRange).highlight(nameLoc.getSourceRange());
2571-
}
2582+
case MemberLookupResult::UR_TypeMemberOnInstance:
2583+
diagnoseTypeMemberOnInstanceLookup(baseObjTy, baseExpr,
2584+
memberName, nameLoc,
2585+
member, loc);
25722586
return;
25732587

25742588
case MemberLookupResult::UR_MutatingMemberOnRValue:

‎test/Constraints/members.swift

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,9 @@ var wcurried1 = w.curried
120120
var wcurried2 = w.curried(0)
121121
var wcurriedFull : () = w.curried(0)(1)
122122

123-
// Member of enum Type
123+
// Member of enum type
124124
func enumMetatypeMember(_ opt: Int?) {
125-
opt.none // expected-error{{static member 'none' cannot be used on instance of type 'Int?'}}
125+
opt.none // expected-error{{enum element 'none' cannot be referenced as an instance member}}
126126
}
127127

128128
////
@@ -310,6 +310,10 @@ protocol StaticP {
310310
extension StaticP {
311311
func bar() {
312312
_ = StaticP.foo(a:) // expected-error{{static member 'foo(a:)' cannot be used on protocol metatype 'StaticP.Protocol'}} {{9-16=Self}}
313+
314+
func nested() {
315+
_ = StaticP.foo(a:) // expected-error{{static member 'foo(a:)' cannot be used on protocol metatype 'StaticP.Protocol'}} {{11-18=Self}}
316+
}
313317
}
314318
}
315319

‎test/Parse/enum.swift

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,49 +453,57 @@ struct SE0036_Auxiliary {}
453453
enum SE0036 {
454454
case A
455455
case B(SE0036_Auxiliary)
456+
case C(SE0036_Auxiliary)
456457

457458
static func staticReference() {
458459
_ = A
460+
_ = self.A
459461
_ = SE0036.A
460462
}
461463

462464
func staticReferenceInInstanceMethod() {
463465
_ = A // expected-error {{enum element 'A' cannot be referenced as an instance member}} {{9-9=SE0036.}}
466+
_ = self.A // expected-error {{enum element 'A' cannot be referenced as an instance member}} {{none}}
464467
_ = SE0036.A
465468
}
466469

467470
static func staticReferenceInSwitchInStaticMethod() {
468471
switch SE0036.A {
469472
case A: break
470473
case B(_): break
474+
case C(let x): _ = x; break
471475
}
472476
}
473477

474478
func staticReferenceInSwitchInInstanceMethod() {
475479
switch self {
476480
case A: break // expected-error {{enum element 'A' cannot be referenced as an instance member}} {{10-10=.}}
477481
case B(_): break // expected-error {{enum element 'B' cannot be referenced as an instance member}} {{10-10=.}}
482+
case C(let x): _ = x; break // expected-error {{invalid pattern}}
478483
}
479484
}
480485

481486
func explicitReferenceInSwitch() {
482487
switch SE0036.A {
483488
case SE0036.A: break
484489
case SE0036.B(_): break
490+
case SE0036.C(let x): _ = x; break
485491
}
486492
}
487493

488494
func dotReferenceInSwitchInInstanceMethod() {
489495
switch self {
490496
case .A: break
491497
case .B(_): break
498+
case .C(let x): _ = x; break
492499
}
493500
}
494501

495502
static func dotReferenceInSwitchInStaticMethod() {
496503
switch SE0036.A {
497504
case .A: break
498505
case .B(_): break
506+
case .C(let x): _ = x; break
499507
}
500508
}
501509

@@ -514,7 +522,11 @@ enum SE0036_Generic<T> {
514522

515523
func foo() {
516524
switch self {
517-
case A(_): break // expected-error {{enum element 'A' cannot be referenced as an instance member}} {{10-10=SE0036_Generic.}}
525+
case A(_): break // expected-error {{enum element 'A' cannot be referenced as an instance member}} {{10-10=.}}
526+
}
527+
528+
switch self {
529+
case .A(let a): print(a)
518530
}
519531

520532
switch self {

‎test/expr/primary/unqualified_name.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ struct S0 {
2929
_ = self.f1(_:`while`:) // expected-warning{{keyword 'while' does not need to be escaped in argument list}}{{19-20=}}{{25-26=}}
3030
_ = self.f2(_:`let`:)
3131

32-
_ = f3(_:y:z:) // expected-error{{static member 'f3(_:y:z:)' cannot be used on instance of type 'S0'}}
32+
_ = f3(_:y:z:) // expected-error{{static member 'f3(_:y:z:)' cannot be used on instance of type 'S0'}}{{9-9=S0.}}
3333
}
3434

3535
static func testStaticS0() {

‎validation-test/compiler_crashers/28376-swift-constraints-constraintsystem-diagnosefailureforexpr.swift renamed to ‎validation-test/compiler_crashers_fixed/28376-swift-constraints-constraintsystem-diagnosefailureforexpr.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
// See http://swift.org/LICENSE.txt for license information
66
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
77

8-
// RUN: not --crash %target-swift-frontend %s -parse
8+
// RUN: not %target-swift-frontend %s -parse
99
// REQUIRES: asserts
1010
class B<enum B{case c
1111
var:{if c

0 commit comments

Comments
 (0)