Skip to content

Commit fb35153

Browse files
committed
[Diagnostics] When checking AssignExpr properly diagnose destination
Currently if destination is unresolved instead of trying to re-typecheck it again and diagnose structural problems which led to such outcome, it gets completely ignored in favor of trying to type-check source without contextual type. That leads to missed diagnostic opportunities, which results in problems on AST verification and SIL generation stages, and generally missleading errors e.g. `.x = 0`. Resolves: SR-3506.
1 parent 0a179db commit fb35153

File tree

6 files changed

+20
-9
lines changed

6 files changed

+20
-9
lines changed

lib/Sema/CSDiag.cpp

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1990,8 +1990,10 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
19901990
TCCOptions options = TCCOptions(),
19911991
ExprTypeCheckListener *listener = nullptr,
19921992
bool allowFreeTypeVariables = true);
1993-
Expr *typeCheckChildIndependently(Expr *subExpr, TCCOptions options) {
1994-
return typeCheckChildIndependently(subExpr, Type(), CTP_Unused, options);
1993+
Expr *typeCheckChildIndependently(Expr *subExpr, TCCOptions options,
1994+
bool allowFreeTypeVariables = true) {
1995+
return typeCheckChildIndependently(subExpr, Type(), CTP_Unused, options,
1996+
nullptr, allowFreeTypeVariables);
19951997
}
19961998

19971999
Type getTypeOfTypeCheckedChildIndependently(Expr *subExpr,
@@ -6050,13 +6052,21 @@ bool FailureDiagnosis::visitAssignExpr(AssignExpr *assignExpr) {
60506052

60516053
auto destType = destExpr->getType();
60526054
if (destType->is<UnresolvedType>() || destType->hasTypeVariable()) {
6053-
// If we have no useful type information from the destination, just type
6055+
// Look closer into why destination has unresolved types since such
6056+
// means that destination has diagnosible structural problems, and it's
6057+
// better to diagnose destination (if possible) before moving on to
6058+
// the source of the assignment.
6059+
destExpr = typeCheckChildIndependently(
6060+
destExpr, TCC_AllowLValue | TCC_ForceRecheck, false);
6061+
if (!destExpr)
6062+
return true;
6063+
6064+
// If re-checking destination didn't produce diagnostic, let's just type
60546065
// check the source without contextual information. If it succeeds, then we
60556066
// win, but if it fails, we'll have to diagnose this another way.
60566067
return !typeCheckChildIndependently(assignExpr->getSrc());
60576068
}
6058-
6059-
6069+
60606070
// If the result type is a non-lvalue, then we are failing because it is
60616071
// immutable and that's not a great thing to assign to.
60626072
if (!destType->isLValueType()) {

test/Constraints/assignment.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,4 @@ func f23798944() {
5757
}
5858
}
5959

60+
.sr_3506 = 0 // expected-error {{reference to member 'sr_3506' cannot be resolved without a contextual type}}
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: not --crash %target-sil-opt %s
1+
// RUN: not %target-sil-opt %s
22
// REQUIRES: asserts
33
.x.a=Int
44
import Swift

validation-test/compiler_crashers/28368-swift-expr-propagatelvalueaccesskind.swift renamed to validation-test/compiler_crashers_fixed/28368-swift-expr-propagatelvalueaccesskind.swift

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

8-
// RUN: not --crash %target-swift-frontend %s -typecheck
8+
// RUN: not %target-swift-frontend %s -typecheck
99
guard let f=.h.a=[]{

validation-test/compiler_crashers/28395-swift-expr-propagatelvalueaccesskind.swift renamed to validation-test/compiler_crashers_fixed/28395-swift-expr-propagatelvalueaccesskind.swift

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

8-
// RUN: not --crash %target-swift-frontend %s -typecheck
8+
// RUN: not %target-swift-frontend %s -typecheck
99
{guard let b=.h.h=.n?(){
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@
55
// See https://swift.org/LICENSE.txt for license information
66
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
77

8-
// RUN: not --crash %target-swift-frontend %s -emit-ir
8+
// RUN: not %target-swift-frontend %s -emit-ir
99
// REQUIRES: asserts
1010
for in.E={return $0 c

0 commit comments

Comments
 (0)