Skip to content

Commit 018498f

Browse files
rudkxrudkx
authored
Merge pull request #16942 from bjhomer/bjhomer/optional-try-flattening
Flatten nested optionals resulting from try? and optional sub-expressions
2 parents 27e53f7 + 510cbd2 commit 018498f

16 files changed

+788
-38
lines changed

‎include/swift/Migrator/ASTMigratorPass.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ void runAPIDiffMigratorPass(EditorAdapter &Editor,
5050
SourceFile *SF,
5151
const MigratorOptions &Opts);
5252

53+
/// Run a pass to fix up the new type of 'try?' in Swift 4
54+
void runOptionalTryMigratorPass(EditorAdapter &Editor,
55+
SourceFile *SF,
56+
const MigratorOptions &Opts);
57+
58+
5359
} // end namespace migrator
5460
} // end namespace swift
5561

‎lib/AST/ASTVerifier.cpp

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1958,15 +1958,21 @@ class Verifier : public ASTWalker {
19581958
void verifyChecked(OptionalTryExpr *E) {
19591959
PrettyStackTraceExpr debugStack(Ctx, "verifying OptionalTryExpr", E);
19601960

1961-
Type unwrappedType = E->getType()->getOptionalObjectType();
1962-
if (!unwrappedType) {
1963-
Out << "OptionalTryExpr result type is not optional\n";
1964-
abort();
1961+
if (Ctx.LangOpts.isSwiftVersionAtLeast(5)) {
1962+
checkSameType(E->getType(), E->getSubExpr()->getType(),
1963+
"OptionalTryExpr and sub-expression");
19651964
}
1966-
1967-
checkSameType(unwrappedType, E->getSubExpr()->getType(),
1968-
"OptionalTryExpr and sub-expression");
1969-
1965+
else {
1966+
Type unwrappedType = E->getType()->getOptionalObjectType();
1967+
if (!unwrappedType) {
1968+
Out << "OptionalTryExpr result type is not optional\n";
1969+
abort();
1970+
}
1971+
1972+
checkSameType(unwrappedType, E->getSubExpr()->getType(),
1973+
"OptionalTryExpr and sub-expression");
1974+
}
1975+
19701976
verifyCheckedBase(E);
19711977
}
19721978

‎lib/AST/Expr.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1821,6 +1821,13 @@ RebindSelfInConstructorExpr::getCalledConstructor(bool &isChainToSuper) const {
18211821
candidate = covariantExpr->getSubExpr();
18221822
continue;
18231823
}
1824+
1825+
// Look through inject into optional expressions
1826+
if (auto injectIntoOptionalExpr
1827+
= dyn_cast<InjectIntoOptionalExpr>(candidate)) {
1828+
candidate = injectIntoOptionalExpr->getSubExpr();
1829+
continue;
1830+
}
18241831
break;
18251832
}
18261833

‎lib/Migrator/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ add_swift_host_library(swiftMigrator STATIC
4848
FixitApplyDiagnosticConsumer.cpp
4949
Migrator.cpp
5050
MigrationState.cpp
51+
OptionalTryMigratorPass.cpp
5152
RewriteBufferEditsReceiver.cpp
5253
LINK_LIBRARIES swiftSyntax swiftIDE)
5354

‎lib/Migrator/Migrator.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,8 @@ bool Migrator::performSyntacticPasses() {
197197

198198
runAPIDiffMigratorPass(Editor, StartInstance->getPrimarySourceFile(),
199199
getMigratorOptions());
200+
runOptionalTryMigratorPass(Editor, StartInstance->getPrimarySourceFile(),
201+
getMigratorOptions());
200202

201203
Edits.commit(Editor.getEdits());
202204

‎lib/Migrator/OptionalTryMigratorPass.cpp

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
//===--- OptionalTryMigratorPass.cpp -------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2018 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
#include "swift/AST/ASTVisitor.h"
14+
#include "swift/AST/Expr.h"
15+
#include "swift/AST/Module.h"
16+
#include "swift/AST/NameLookup.h"
17+
#include "swift/AST/ParameterList.h"
18+
#include "swift/AST/Types.h"
19+
#include "swift/IDE/SourceEntityWalker.h"
20+
#include "swift/Migrator/ASTMigratorPass.h"
21+
#include "swift/Parse/Lexer.h"
22+
23+
using namespace swift;
24+
using namespace swift::migrator;
25+
26+
namespace {
27+
28+
class OptionalTryMigratorPass: public ASTMigratorPass,
29+
public SourceEntityWalker {
30+
31+
bool explicitCastActiveForOptionalTry = false;
32+
33+
bool walkToExprPre(Expr *E) override {
34+
if (dyn_cast<ParenExpr>(E) || E->isImplicit()) {
35+
// Look through parentheses and implicit expressions.
36+
return true;
37+
}
38+
39+
if (const auto *explicitCastExpr = dyn_cast<ExplicitCastExpr>(E)) {
40+
// If the user has already provided an explicit cast for the
41+
// 'try?', then we don't need to add one. So let's track whether
42+
// one is active
43+
explicitCastActiveForOptionalTry = true;
44+
}
45+
else if (const auto *optTryExpr = dyn_cast<OptionalTryExpr>(E)) {
46+
wrapTryInCastIfNeeded(optTryExpr);
47+
return false;
48+
}
49+
else if (explicitCastActiveForOptionalTry) {
50+
// If an explicit cast is active and we are entering a new
51+
// expression that is not an OptionalTryExpr, then the cast
52+
// does not apply to the OptionalTryExpr.
53+
explicitCastActiveForOptionalTry = false;
54+
}
55+
return true;
56+
}
57+
58+
bool walkToExprPost(Expr *E) override {
59+
explicitCastActiveForOptionalTry = false;
60+
return true;
61+
}
62+
63+
void wrapTryInCastIfNeeded(const OptionalTryExpr *optTryExpr) {
64+
if (explicitCastActiveForOptionalTry) {
65+
// There's already an explicit cast here; we don't need to add anything
66+
return;
67+
}
68+
69+
if (!optTryExpr->getSubExpr()->getType()->getOptionalObjectType()) {
70+
// This 'try?' doesn't wrap an optional, so its behavior does not
71+
// change from Swift 4 to Swift 5
72+
return;
73+
}
74+
75+
Type typeToPreserve = optTryExpr->getType();
76+
auto typeName = typeToPreserve->getStringAsComponent();
77+
78+
auto range = optTryExpr->getSourceRange();
79+
auto charRange = Lexer::getCharSourceRangeFromSourceRange(SM, range);
80+
Editor.insertWrap("((", charRange, (Twine(") as ") + typeName + ")").str());
81+
}
82+
83+
public:
84+
OptionalTryMigratorPass(EditorAdapter &Editor,
85+
SourceFile *SF,
86+
const MigratorOptions &Opts)
87+
: ASTMigratorPass(Editor, SF, Opts) {}
88+
};
89+
90+
} // end anonymous namespace
91+
92+
void migrator::runOptionalTryMigratorPass(EditorAdapter &Editor,
93+
SourceFile *SF,
94+
const MigratorOptions &Opts) {
95+
OptionalTryMigratorPass { Editor, SF, Opts }.walk(SF);
96+
}

‎lib/SILGen/SILGenExpr.cpp

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1082,6 +1082,9 @@ RValue RValueEmitter::visitForceTryExpr(ForceTryExpr *E, SGFContext C) {
10821082
RValue RValueEmitter::visitOptionalTryExpr(OptionalTryExpr *E, SGFContext C) {
10831083
// FIXME: Much of this was copied from visitOptionalEvaluationExpr.
10841084

1085+
// Prior to Swift 5, an optional try's subexpression is always wrapped in an additional optional
1086+
bool shouldWrapInOptional = !(SGF.getASTContext().LangOpts.isSwiftVersionAtLeast(5));
1087+
10851088
auto &optTL = SGF.getTypeLowering(E->getType());
10861089

10871090
Initialization *optInit = C.getEmitInto();
@@ -1112,16 +1115,27 @@ RValue RValueEmitter::visitOptionalTryExpr(OptionalTryExpr *E, SGFContext C) {
11121115
JumpDest(catchBB, SGF.Cleanups.getCleanupsDepth(), E)};
11131116

11141117
SILValue branchArg;
1115-
if (isByAddress) {
1116-
assert(optInit);
1117-
SILValue optAddr = optInit->getAddressForInPlaceInitialization(SGF, E);
1118-
SGF.emitInjectOptionalValueInto(E, E->getSubExpr(), optAddr, optTL);
1119-
} else {
1120-
ManagedValue subExprValue = SGF.emitRValueAsSingleValue(E->getSubExpr());
1121-
ManagedValue wrapped = SGF.getOptionalSomeValue(E, subExprValue, optTL);
1122-
branchArg = wrapped.forward(SGF);
1118+
if (shouldWrapInOptional) {
1119+
if (isByAddress) {
1120+
assert(optInit);
1121+
SILValue optAddr = optInit->getAddressForInPlaceInitialization(SGF, E);
1122+
SGF.emitInjectOptionalValueInto(E, E->getSubExpr(), optAddr, optTL);
1123+
} else {
1124+
ManagedValue subExprValue = SGF.emitRValueAsSingleValue(E->getSubExpr());
1125+
ManagedValue wrapped = SGF.getOptionalSomeValue(E, subExprValue, optTL);
1126+
branchArg = wrapped.forward(SGF);
1127+
}
11231128
}
1124-
1129+
else {
1130+
if (isByAddress) {
1131+
assert(optInit);
1132+
SGF.emitExprInto(E->getSubExpr(), optInit);
1133+
} else {
1134+
ManagedValue subExprValue = SGF.emitRValueAsSingleValue(E->getSubExpr());
1135+
branchArg = subExprValue.forward(SGF);
1136+
}
1137+
}
1138+
11251139
localCleanups.pop();
11261140

11271141
// If it turns out there are no uses of the catch block, just drop it.
@@ -1133,8 +1147,10 @@ RValue RValueEmitter::visitOptionalTryExpr(OptionalTryExpr *E, SGFContext C) {
11331147
if (!isByAddress)
11341148
return RValue(SGF, E,
11351149
SGF.emitManagedRValueWithCleanup(branchArg, optTL));
1136-
1137-
optInit->finishInitialization(SGF);
1150+
1151+
if (shouldWrapInOptional) {
1152+
optInit->finishInitialization(SGF);
1153+
}
11381154

11391155
// If we emitted into the provided context, we're done.
11401156
if (usingProvidedContext)
@@ -1181,8 +1197,10 @@ RValue RValueEmitter::visitOptionalTryExpr(OptionalTryExpr *E, SGFContext C) {
11811197
return RValue(SGF, E, SGF.emitManagedRValueWithCleanup(arg, optTL));
11821198
}
11831199

1184-
optInit->finishInitialization(SGF);
1185-
1200+
if (shouldWrapInOptional) {
1201+
optInit->finishInitialization(SGF);
1202+
}
1203+
11861204
// If we emitted into the provided context, we're done.
11871205
if (usingProvidedContext)
11881206
return RValue::forInContext();

‎lib/Sema/CSApply.cpp

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2810,7 +2810,38 @@ namespace {
28102810
}
28112811

28122812
Expr *visitOptionalTryExpr(OptionalTryExpr *expr) {
2813-
return simplifyExprType(expr);
2813+
// Prior to Swift 5, 'try?' simply wraps the type of its sub-expression
2814+
// in an Optional, regardless of the sub-expression type.
2815+
//
2816+
// In Swift 5+, the type of a 'try?' expression of static type T is:
2817+
// - Equal to T if T is optional
2818+
// - Equal to T? if T is not optional
2819+
//
2820+
// The result is that in Swift 5, 'try?' avoids producing nested optionals.
2821+
2822+
if (!cs.getTypeChecker().getLangOpts().isSwiftVersionAtLeast(5)) {
2823+
// Nothing to do for Swift 4 and earlier!
2824+
return simplifyExprType(expr);
2825+
}
2826+
2827+
Type subExprType = cs.getType(expr->getSubExpr());
2828+
Type targetType = simplifyType(subExprType);
2829+
2830+
// If the subexpression is not optional, wrap it in
2831+
// an InjectIntoOptionalExpr. Then use the type of the
2832+
// subexpression as the type of the 'try?' expr
2833+
bool subExprIsOptional = (bool) subExprType->getOptionalObjectType();
2834+
2835+
if (!subExprIsOptional) {
2836+
targetType = OptionalType::get(targetType);
2837+
auto subExpr = coerceToType(expr->getSubExpr(), targetType,
2838+
cs.getConstraintLocator(expr));
2839+
if (!subExpr) return nullptr;
2840+
expr->setSubExpr(subExpr);
2841+
}
2842+
2843+
cs.setType(expr, targetType);
2844+
return expr;
28142845
}
28152846

28162847
Expr *visitParenExpr(ParenExpr *expr) {

‎lib/Sema/CSDiagnostics.cpp

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -576,10 +576,22 @@ bool MissingOptionalUnwrapFailure::diagnoseAsError() {
576576
auto *tryExpr = dyn_cast<OptionalTryExpr>(unwrapped);
577577
if (!tryExpr)
578578
return diagnoseUnwrap(getConstraintSystem(), unwrapped, type);
579-
580-
emitDiagnostic(tryExpr->getTryLoc(), diag::missing_unwrap_optional_try, type)
581-
.fixItReplace({tryExpr->getTryLoc(), tryExpr->getQuestionLoc()}, "try!");
582-
return true;
579+
580+
bool isSwift5OrGreater = getTypeChecker().getLangOpts().isSwiftVersionAtLeast(5);
581+
auto subExprType = getType(tryExpr->getSubExpr());
582+
bool subExpressionIsOptional = (bool)subExprType->getOptionalObjectType();
583+
584+
585+
if (isSwift5OrGreater && subExpressionIsOptional) {
586+
// Using 'try!' won't change the type for a 'try?' with an optional sub-expr
587+
// under Swift 5+, so just report that a missing unwrap can't be handled here.
588+
return false;
589+
}
590+
else {
591+
emitDiagnostic(tryExpr->getTryLoc(), diag::missing_unwrap_optional_try, type)
592+
.fixItReplace({tryExpr->getTryLoc(), tryExpr->getQuestionLoc()}, "try!");
593+
return true;
594+
}
583595
}
584596

585597
bool RValueTreatedAsLValueFailure::diagnoseAsError() {

‎lib/Sema/CSGen.cpp

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1709,9 +1709,18 @@ namespace {
17091709
if (!optTy)
17101710
return Type();
17111711

1712-
CS.addConstraint(ConstraintKind::OptionalObject,
1713-
optTy, CS.getType(expr->getSubExpr()),
1714-
CS.getConstraintLocator(expr));
1712+
// Prior to Swift 5, 'try?' always adds an additional layer of optionality,
1713+
// even if the sub-expression was already optional.
1714+
if (CS.getTypeChecker().getLangOpts().isSwiftVersionAtLeast(5)) {
1715+
CS.addConstraint(ConstraintKind::Conversion,
1716+
CS.getType(expr->getSubExpr()), optTy,
1717+
CS.getConstraintLocator(expr));
1718+
}
1719+
else {
1720+
CS.addConstraint(ConstraintKind::OptionalObject,
1721+
optTy, CS.getType(expr->getSubExpr()),
1722+
CS.getConstraintLocator(expr));
1723+
}
17151724
return optTy;
17161725
}
17171726

‎lib/Sema/CSSimplify.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2401,6 +2401,20 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
24012401
forceUnwrapPossible = false;
24022402
}
24032403
}
2404+
2405+
if (auto optTryExpr =
2406+
dyn_cast_or_null<OptionalTryExpr>(locator.trySimplifyToExpr())) {
2407+
auto subExprType = getType(optTryExpr->getSubExpr());
2408+
bool isSwift5OrGreater = TC.getLangOpts().isSwiftVersionAtLeast(5);
2409+
if (isSwift5OrGreater && (bool)subExprType->getOptionalObjectType()) {
2410+
// For 'try?' expressions, a ForceOptional fix converts 'try?'
2411+
// to 'try!'. If the sub-expression is optional, then a force-unwrap
2412+
// won't change anything in Swift 5+ because 'try?' already avoids
2413+
// adding an additional layer of Optional there.
2414+
forceUnwrapPossible = false;
2415+
}
2416+
}
2417+
24042418

24052419
if (forceUnwrapPossible) {
24062420
conversionsOrFixes.push_back(

0 commit comments

Comments
 (0)