Skip to content

Commit d2720e3

Browse files
authored
Merge pull request #12516 from gregomni/6175
[Sema] Improve subscript diagnostics in assignment destinations
2 parents 5768bd1 + 18a113e commit d2720e3

File tree

2 files changed

+65
-8
lines changed

2 files changed

+65
-8
lines changed

lib/Sema/CSDiag.cpp

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -987,6 +987,8 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
987987
bool diagnoseClosureExpr(ClosureExpr *closureExpr, Type contextualType,
988988
std::function<bool(Type, Type)> resultTypeProcessor);
989989

990+
bool diagnoseSubscriptErrors(SubscriptExpr *SE, bool performingSet);
991+
990992
bool visitExpr(Expr *E);
991993
bool visitIdentityExpr(IdentityExpr *E);
992994
bool visitTryExpr(TryExpr *E);
@@ -4478,9 +4480,7 @@ bool FailureDiagnosis::diagnoseParameterErrors(CalleeCandidateInfo &CCI,
44784480
return false;
44794481
}
44804482

4481-
4482-
4483-
bool FailureDiagnosis::visitSubscriptExpr(SubscriptExpr *SE) {
4483+
bool FailureDiagnosis::diagnoseSubscriptErrors(SubscriptExpr *SE, bool inAssignmentDestination) {
44844484
auto baseExpr = typeCheckChildIndependently(SE->getBase());
44854485
if (!baseExpr) return true;
44864486
auto baseType = CS.getType(baseExpr);
@@ -4494,8 +4494,7 @@ bool FailureDiagnosis::visitSubscriptExpr(SubscriptExpr *SE) {
44944494
std::function<bool(ArrayRef<OverloadChoice>)> callback =
44954495
[&](ArrayRef<OverloadChoice> candidates) -> bool {
44964496
CalleeCandidateInfo calleeInfo(Type(), candidates, SE->hasTrailingClosure(),
4497-
CS,
4498-
/*selfAlreadyApplied*/ false);
4497+
CS, /*selfAlreadyApplied*/ false);
44994498

45004499
// We're about to typecheck the index list, which needs to be processed with
45014500
// self already applied.
@@ -4522,7 +4521,9 @@ bool FailureDiagnosis::visitSubscriptExpr(SubscriptExpr *SE) {
45224521
calleeInfo.filterList(
45234522
[&](UncurriedCandidate cand) -> CalleeCandidateInfo::ClosenessResultTy {
45244523
// Classify how close this match is. Non-subscript decls don't match.
4525-
if (!dyn_cast_or_null<SubscriptDecl>(cand.getDecl()))
4524+
auto subscriptDecl = dyn_cast_or_null<SubscriptDecl>(cand.getDecl());
4525+
if (!subscriptDecl ||
4526+
(inAssignmentDestination && !subscriptDecl->isSettable()))
45264527
return {CC_GeneralMismatch, {}};
45274528

45284529
// Check whether the self type matches.
@@ -4544,8 +4545,7 @@ bool FailureDiagnosis::visitSubscriptExpr(SubscriptExpr *SE) {
45444545
});
45454546

45464547
// If the closest matches all mismatch on self, we either have something
4547-
// that
4548-
// cannot be subscripted, or an ambiguity.
4548+
// that cannot be subscripted, or an ambiguity.
45494549
if (calleeInfo.closeness == CC_SelfMismatch) {
45504550
diagnose(SE->getLoc(), diag::cannot_subscript_base, baseType)
45514551
.highlight(SE->getBase()->getSourceRange());
@@ -4575,6 +4575,20 @@ bool FailureDiagnosis::visitSubscriptExpr(SubscriptExpr *SE) {
45754575

45764576
if (CS.TC.getTypeOfExpressionWithoutApplying(expr, CS.DC, decl))
45774577
return false;
4578+
4579+
// If we are down to a single candidate but with an unresolved
4580+
// index type, we can substitute in the base type to get a simpler
4581+
// and more concrete expected type for this subscript decl, in order
4582+
// to diagnose a better error.
4583+
if (baseType && indexType->hasUnresolvedType()) {
4584+
UncurriedCandidate cand = calleeInfo.candidates[0];
4585+
auto candType = baseType->getTypeOfMember(CS.DC->getParentModule(),
4586+
cand.getDecl(), nullptr);
4587+
auto paramsType = candType->getAs<FunctionType>()->getInput();
4588+
if (!typeCheckChildIndependently(indexExpr, paramsType,
4589+
CTP_CallArgument, TCC_ForceRecheck))
4590+
return true;
4591+
}
45784592
}
45794593

45804594
diagnose(SE->getLoc(), message, baseType, indexType)
@@ -4599,6 +4613,12 @@ bool FailureDiagnosis::visitSubscriptExpr(SubscriptExpr *SE) {
45994613
if (calleeInfo.diagnoseSimpleErrors(SE))
46004614
return true;
46014615

4616+
// If we haven't found a diagnostic yet, and we are in an assignment's
4617+
// destination, continue with diagnosing the assignment rather than giving
4618+
// a last resort diagnostic here.
4619+
if (inAssignmentDestination)
4620+
return false;
4621+
46024622
diagnose(SE->getLoc(), diag::cannot_subscript_with_index, baseType,
46034623
indexType);
46044624

@@ -4615,6 +4635,9 @@ bool FailureDiagnosis::visitSubscriptExpr(SubscriptExpr *SE) {
46154635
callback);
46164636
}
46174637

4638+
bool FailureDiagnosis::visitSubscriptExpr(SubscriptExpr *SE) {
4639+
return diagnoseSubscriptErrors(SE, /* inAssignmentDestination = */ false);
4640+
}
46184641

46194642
namespace {
46204643
/// Type checking listener for pattern binding initializers.
@@ -5833,6 +5856,14 @@ bool FailureDiagnosis::visitAssignExpr(AssignExpr *assignExpr) {
58335856
// If the result type is a non-lvalue, then we are failing because it is
58345857
// immutable and that's not a great thing to assign to.
58355858
if (!destType->hasLValueType()) {
5859+
// If the destination is a subscript, the problem may actually be that we
5860+
// incorrectly decided on a get-only subscript overload, and we may be able
5861+
// to come up with a better diagnosis by looking only at subscript candidates
5862+
// that are set-able.
5863+
if (auto subscriptExpr = dyn_cast<SubscriptExpr>(destExpr)) {
5864+
if (diagnoseSubscriptErrors(subscriptExpr, /* inAssignmentDestination = */ true))
5865+
return true;
5866+
}
58365867
CS.diagnoseAssignmentFailure(destExpr, destType, assignExpr->getLoc());
58375868
return true;
58385869
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// RUN: %target-typecheck-verify-swift
2+
3+
enum Key: Int {
4+
case aKey
5+
case anotherKey // expected-note {{did you mean 'anotherKey'?}}
6+
}
7+
8+
class sr6175 {
9+
var dict: [Key: String] = [:]
10+
func what() -> Void {
11+
dict[.notAKey] = "something" // expected-error {{type 'Key' has no member 'notAKey'}}
12+
}
13+
14+
subscript(i: Int) -> Int {
15+
return i*i
16+
}
17+
subscript(j: Double) -> Double {
18+
get { return j*j }
19+
set {}
20+
}
21+
}
22+
23+
let s = sr6175()
24+
let one: Int = 1
25+
// Should choose the settable subscript to find a problem with, not the get-only subscript
26+
s[one] = 2.5 // expected-error {{cannot convert value of type 'Int' to expected argument type 'Double'}}

0 commit comments

Comments
 (0)