Skip to content

Commit 5d1fea2

Browse files
committed
[CS] Don't consider implicit TupleExprs in exprNeedsParensOutsideFollowingOperator
Such implicit tuples may be used to represent argument lists for e.g binary expressions, and as such shouldn't be considered as parent exprs that satisfy the role of parentheses. Also fix the callers to use the raw anchor as the root expression they pass to provide an accurate parent map. This requires sinking the UnresolvedMemberChainResultExpr handling logic into `getPrecedenceParentAndIndex`. rdar://81109287
1 parent 41a1761 commit 5d1fea2

File tree

4 files changed

+46
-11
lines changed

4 files changed

+46
-11
lines changed

lib/Sema/CSApply.cpp

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7891,6 +7891,15 @@ static std::pair<Expr *, unsigned> getPrecedenceParentAndIndex(Expr *expr,
78917891
}
78927892
Expr *parent = it->second;
78937893

7894+
// Look through an unresolved chain wrapper expr, as it has no effect on
7895+
// precedence.
7896+
if (isa<UnresolvedMemberChainResultExpr>(parent)) {
7897+
it = parentMap.find(parent);
7898+
if (it == parentMap.end())
7899+
return {nullptr, 0};
7900+
parent = it->second;
7901+
}
7902+
78947903
// Handle all cases where the answer isn't just going to be { parent, 0 }.
78957904
if (auto tuple = dyn_cast<TupleExpr>(parent)) {
78967905
// Get index of expression in tuple.
@@ -7970,13 +7979,13 @@ bool swift::exprNeedsParensOutsideFollowingOperator(
79707979
Expr *parent;
79717980
unsigned index;
79727981
std::tie(parent, index) = getPrecedenceParentAndIndex(expr, rootExpr);
7973-
if (!parent || isa<TupleExpr>(parent)) {
7982+
if (!parent)
79747983
return false;
7975-
}
79767984

7977-
if (auto parenExp = dyn_cast<ParenExpr>(parent))
7978-
if (!parenExp->isImplicit())
7985+
if (isa<ParenExpr>(parent) || isa<TupleExpr>(parent)) {
7986+
if (!parent->isImplicit())
79797987
return false;
7988+
}
79807989

79817990
if (parent->isInfixOperator()) {
79827991
auto parentPG = TypeChecker::lookupPrecedenceGroupForInfixOperator(DC,

lib/Sema/CSDiagnostics.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1146,6 +1146,7 @@ ASTNode MissingExplicitConversionFailure::getAnchor() const {
11461146
bool MissingExplicitConversionFailure::diagnoseAsError() {
11471147
auto *DC = getDC();
11481148
auto *anchor = castToExpr(getAnchor());
1149+
auto *rawAnchor = castToExpr(getRawAnchor());
11491150

11501151
auto fromType = getFromType();
11511152
auto toType = getToType();
@@ -1171,7 +1172,7 @@ bool MissingExplicitConversionFailure::diagnoseAsError() {
11711172
}
11721173

11731174
bool needsParensInside = exprNeedsParensBeforeAddingAs(anchor);
1174-
bool needsParensOutside = exprNeedsParensAfterAddingAs(anchor, expr);
1175+
bool needsParensOutside = exprNeedsParensAfterAddingAs(anchor, rawAnchor);
11751176

11761177
llvm::SmallString<2> insertBefore;
11771178
llvm::SmallString<32> insertAfter;
@@ -1310,11 +1311,8 @@ void MissingOptionalUnwrapFailure::offerDefaultValueUnwrapFixIt(
13101311
// Figure out what we need to parenthesize.
13111312
bool needsParensInside =
13121313
exprNeedsParensBeforeAddingNilCoalescing(DC, const_cast<Expr *>(expr));
1313-
auto parentExpr = findParentExpr(anchor);
1314-
if (parentExpr && isa<UnresolvedMemberChainResultExpr>(parentExpr))
1315-
parentExpr = findParentExpr(parentExpr);
13161314
bool needsParensOutside = exprNeedsParensAfterAddingNilCoalescing(
1317-
DC, const_cast<Expr *>(expr), parentExpr);
1315+
DC, const_cast<Expr *>(expr), castToExpr(getRawAnchor()));
13181316

13191317
llvm::SmallString<2> insertBefore;
13201318
llvm::SmallString<32> insertAfter;

test/Constraints/bridging.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -258,9 +258,9 @@ func rdar19770981(_ s: String, ns: NSString) {
258258
_ = ns as String > s
259259

260260
// 'as' has lower precedence than '+' so add parens with the fixit:
261-
s + ns // expected-error{{'NSString' is not implicitly convertible to 'String'; did you mean to use 'as' to explicitly convert?}}{{9-9= as String}}
261+
s + ns // expected-error{{'NSString' is not implicitly convertible to 'String'; did you mean to use 'as' to explicitly convert?}} {{7-7=(}} {{9-9= as String)}}
262262
_ = s + (ns as String)
263-
ns + s // expected-error{{'NSString' is not implicitly convertible to 'String'; did you mean to use 'as' to explicitly convert?}}{{5-5= as String}}
263+
ns + s // expected-error{{'NSString' is not implicitly convertible to 'String'; did you mean to use 'as' to explicitly convert?}} {{3-3=(}} {{5-5= as String)}}
264264
_ = (ns as String) + s
265265
}
266266

test/Constraints/diagnostics.swift

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1415,3 +1415,31 @@ func rdar74696023() {
14151415
// expected-note@-2 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}}
14161416
}
14171417
}
1418+
1419+
func testUnwrapFixIts(x: Int?) {
1420+
let _ = x + 2 // expected-error {{value of optional type 'Int?' must be unwrapped to a value of type 'Int'}}
1421+
// expected-note@-1 {{coalesce using '??' to provide a default when the optional value contains 'nil'}} {{11-11=(}} {{12-12= ?? <#default value#>)}}
1422+
// expected-note@-2 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}} {{12-12=!}}
1423+
let _ = (x ?? 0) + 2
1424+
1425+
let _ = 2 + x // expected-error {{value of optional type 'Int?' must be unwrapped to a value of type 'Int'}}
1426+
// expected-note@-1 {{coalesce using '??' to provide a default when the optional value contains 'nil'}} {{15-15=(}} {{16-16= ?? <#default value#>)}}
1427+
// expected-note@-2 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}} {{16-16=!}}
1428+
let _ = 2 + (x ?? 0)
1429+
1430+
func foo(y: Int) {}
1431+
foo(y: x) // expected-error {{value of optional type 'Int?' must be unwrapped to a value of type 'Int'}}
1432+
// expected-note@-1 {{coalesce using '??' to provide a default when the optional value contains 'nil'}} {{11-11= ?? <#default value#>}}
1433+
// expected-note@-2 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}} {{11-11=!}}
1434+
foo(y: x ?? 0)
1435+
1436+
let _ = x < 2 // expected-error {{value of optional type 'Int?' must be unwrapped to a value of type 'Int'}}
1437+
// expected-note@-1 {{coalesce using '??' to provide a default when the optional value contains 'nil'}} {{12-12= ?? <#default value#>}}
1438+
// expected-note@-2 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}} {{12-12=!}}
1439+
let _ = x ?? 0 < 2
1440+
1441+
let _ = 2 < x // expected-error {{value of optional type 'Int?' must be unwrapped to a value of type 'Int'}}
1442+
// expected-note@-1 {{coalesce using '??' to provide a default when the optional value contains 'nil'}} {{16-16= ?? <#default value#>}}
1443+
// expected-note@-2 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}} {{16-16=!}}
1444+
let _ = 2 < x ?? 0
1445+
}

0 commit comments

Comments
 (0)