Flatten nested optionals resulting from try? and optional sub-expressions

include/swift/Migrator/ASTMigratorPass.h

@@ -50,6 +50,12 @@ void runAPIDiffMigratorPass(EditorAdapter &Editor,
                             SourceFile *SF,
                             const MigratorOptions &Opts);
 
+/// Run a pass to fix up the new type of 'try?' in Swift 4
+void runOptionalTryMigratorPass(EditorAdapter &Editor,
+                                SourceFile *SF,
+                                const MigratorOptions &Opts);
+
+
 } // end namespace migrator
 } // end namespace swift
 

lib/AST/ASTVerifier.cpp

@@ -1958,15 +1958,21 @@ class Verifier : public ASTWalker {
     void verifyChecked(OptionalTryExpr *E) {
       PrettyStackTraceExpr debugStack(Ctx, "verifying OptionalTryExpr", E);
 
-      Type unwrappedType = E->getType()->getOptionalObjectType();
-      if (!unwrappedType) {
-        Out << "OptionalTryExpr result type is not optional\n";
-        abort();
+      if (Ctx.LangOpts.isSwiftVersionAtLeast(5)) {
+        checkSameType(E->getType(), E->getSubExpr()->getType(),
+                      "OptionalTryExpr and sub-expression");
       }
-
-      checkSameType(unwrappedType, E->getSubExpr()->getType(),
-                    "OptionalTryExpr and sub-expression");
-
+      else {
+        Type unwrappedType = E->getType()->getOptionalObjectType();
+        if (!unwrappedType) {
+          Out << "OptionalTryExpr result type is not optional\n";
+          abort();
+        }
+
+        checkSameType(unwrappedType, E->getSubExpr()->getType(),
+                      "OptionalTryExpr and sub-expression");
+      }
+
       verifyCheckedBase(E);
     }
 

lib/AST/Expr.cpp

@@ -1821,6 +1821,13 @@ RebindSelfInConstructorExpr::getCalledConstructor(bool &isChainToSuper) const {
       candidate = covariantExpr->getSubExpr();
       continue;
     }
+
+    // Look through inject into optional expressions
+    if (auto injectIntoOptionalExpr
+        = dyn_cast<InjectIntoOptionalExpr>(candidate)) {
+      candidate = injectIntoOptionalExpr->getSubExpr();
+      continue;
+    }
     break;
   }
 

lib/Migrator/CMakeLists.txt

@@ -48,6 +48,7 @@ add_swift_host_library(swiftMigrator STATIC
   FixitApplyDiagnosticConsumer.cpp
   Migrator.cpp
   MigrationState.cpp
+  OptionalTryMigratorPass.cpp
   RewriteBufferEditsReceiver.cpp
   LINK_LIBRARIES swiftSyntax swiftIDE)
 

lib/Migrator/Migrator.cpp

@@ -197,6 +197,8 @@ bool Migrator::performSyntacticPasses() {
 
   runAPIDiffMigratorPass(Editor, StartInstance->getPrimarySourceFile(),
                          getMigratorOptions());
+  runOptionalTryMigratorPass(Editor, StartInstance->getPrimarySourceFile(),
+                        getMigratorOptions());
 
   Edits.commit(Editor.getEdits());
 

lib/Migrator/OptionalTryMigratorPass.cpp

@@ -0,0 +1,96 @@
+//===--- OptionalTryMigratorPass.cpp -------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2018 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+#include "swift/AST/ASTVisitor.h"
+#include "swift/AST/Expr.h"
+#include "swift/AST/Module.h"
+#include "swift/AST/NameLookup.h"
+#include "swift/AST/ParameterList.h"
+#include "swift/AST/Types.h"
+#include "swift/IDE/SourceEntityWalker.h"
+#include "swift/Migrator/ASTMigratorPass.h"
+#include "swift/Parse/Lexer.h"
+
+using namespace swift;
+using namespace swift::migrator;
+
+namespace {
+
+  class OptionalTryMigratorPass: public ASTMigratorPass,
+  public SourceEntityWalker {
+
+    bool explicitCastActiveForOptionalTry = false;
+
+    bool walkToExprPre(Expr *E) override {
+      if (dyn_cast<ParenExpr>(E) || E->isImplicit()) {
+        // Look through parentheses and implicit expressions.
+        return true;
+      }
+
+      if (const auto *explicitCastExpr = dyn_cast<ExplicitCastExpr>(E)) {
+        // If the user has already provided an explicit cast for the
+        // 'try?', then we don't need to add one. So let's track whether
+        // one is active
+        explicitCastActiveForOptionalTry = true;
+      }
+      else if (const auto *optTryExpr = dyn_cast<OptionalTryExpr>(E)) {
+        wrapTryInCastIfNeeded(optTryExpr);
+        return false;
+      }
+      else if (explicitCastActiveForOptionalTry) {
+        // If an explicit cast is active and we are entering a new
+        // expression that is not an OptionalTryExpr, then the cast
+        // does not apply to the OptionalTryExpr.
+        explicitCastActiveForOptionalTry = false;
+      }
+      return true;
+    }
+
+    bool walkToExprPost(Expr *E) override {
+      explicitCastActiveForOptionalTry = false;
+      return true;
+    }
+
+    void wrapTryInCastIfNeeded(const OptionalTryExpr *optTryExpr) {
+      if (explicitCastActiveForOptionalTry) {
+        // There's already an explicit cast here; we don't need to add anything
+        return;
+      }
+
+      if (!optTryExpr->getSubExpr()->getType()->getOptionalObjectType()) {
+        // This 'try?' doesn't wrap an optional, so its behavior does not
+        // change from Swift 4 to Swift 5
+        return;
+      }
+
+      Type typeToPreserve = optTryExpr->getType();
+      auto typeName = typeToPreserve->getStringAsComponent();
+
+      auto range = optTryExpr->getSourceRange();
+      auto charRange = Lexer::getCharSourceRangeFromSourceRange(SM, range);
+      Editor.insertWrap("((", charRange, (Twine(") as ") + typeName + ")").str());
+    }
+
+  public:
+    OptionalTryMigratorPass(EditorAdapter &Editor,
+                       SourceFile *SF,
+                       const MigratorOptions &Opts)
+    : ASTMigratorPass(Editor, SF, Opts) {}
+  };
+
+} // end anonymous namespace
+
+void migrator::runOptionalTryMigratorPass(EditorAdapter &Editor,
+                                     SourceFile *SF,
+                                     const MigratorOptions &Opts) {
+  OptionalTryMigratorPass { Editor, SF, Opts }.walk(SF);
+}

lib/SILGen/SILGenExpr.cpp

@@ -1082,6 +1082,9 @@ RValue RValueEmitter::visitForceTryExpr(ForceTryExpr *E, SGFContext C) {
 RValue RValueEmitter::visitOptionalTryExpr(OptionalTryExpr *E, SGFContext C) {
   // FIXME: Much of this was copied from visitOptionalEvaluationExpr.
 
+  // Prior to Swift 5, an optional try's subexpression is always wrapped in an additional optional
+  bool shouldWrapInOptional = !(SGF.getASTContext().LangOpts.isSwiftVersionAtLeast(5));
+
   auto &optTL = SGF.getTypeLowering(E->getType());
 
   Initialization *optInit = C.getEmitInto();
@@ -1112,16 +1115,27 @@ RValue RValueEmitter::visitOptionalTryExpr(OptionalTryExpr *E, SGFContext C) {
     JumpDest(catchBB, SGF.Cleanups.getCleanupsDepth(), E)};
 
   SILValue branchArg;
-  if (isByAddress) {
-    assert(optInit);
-    SILValue optAddr = optInit->getAddressForInPlaceInitialization(SGF, E);
-    SGF.emitInjectOptionalValueInto(E, E->getSubExpr(), optAddr, optTL);
-  } else {
-    ManagedValue subExprValue = SGF.emitRValueAsSingleValue(E->getSubExpr());
-    ManagedValue wrapped = SGF.getOptionalSomeValue(E, subExprValue, optTL);
-    branchArg = wrapped.forward(SGF);
+  if (shouldWrapInOptional) {
+    if (isByAddress) {
+      assert(optInit);
+      SILValue optAddr = optInit->getAddressForInPlaceInitialization(SGF, E);
+      SGF.emitInjectOptionalValueInto(E, E->getSubExpr(), optAddr, optTL);
+    } else {
+      ManagedValue subExprValue = SGF.emitRValueAsSingleValue(E->getSubExpr());
+      ManagedValue wrapped = SGF.getOptionalSomeValue(E, subExprValue, optTL);
+      branchArg = wrapped.forward(SGF);
+    }
   }
-
+  else {
+    if (isByAddress) {
+      assert(optInit);
+      SGF.emitExprInto(E->getSubExpr(), optInit);
+    } else {
+      ManagedValue subExprValue = SGF.emitRValueAsSingleValue(E->getSubExpr());
+      branchArg = subExprValue.forward(SGF);
+    }
+  }
+
   localCleanups.pop();
 
   // 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) {
     if (!isByAddress)
       return RValue(SGF, E,
                     SGF.emitManagedRValueWithCleanup(branchArg, optTL));
-
-    optInit->finishInitialization(SGF);
+
+    if (shouldWrapInOptional) {
+      optInit->finishInitialization(SGF);
+    }
 
     // If we emitted into the provided context, we're done.
     if (usingProvidedContext)
@@ -1181,8 +1197,10 @@ RValue RValueEmitter::visitOptionalTryExpr(OptionalTryExpr *E, SGFContext C) {
     return RValue(SGF, E, SGF.emitManagedRValueWithCleanup(arg, optTL));
   }
 
-  optInit->finishInitialization(SGF);
-
+  if (shouldWrapInOptional) {
+    optInit->finishInitialization(SGF);
+  }
+
   // If we emitted into the provided context, we're done.
   if (usingProvidedContext)
     return RValue::forInContext();

lib/Sema/CSApply.cpp

@@ -2808,7 +2808,38 @@ namespace {
     }
 
     Expr *visitOptionalTryExpr(OptionalTryExpr *expr) {
-      return simplifyExprType(expr);
+      // Prior to Swift 5, 'try?' simply wraps the type of its sub-expression
+      // in an Optional, regardless of the sub-expression type.
+      //
+      // In Swift 5+, the type of a 'try?' expression of static type T is:
+      //  - Equal to T if T is optional
+      //  - Equal to T? if T is not optional
+      //
+      // The result is that in Swift 5, 'try?' avoids producing nested optionals.
+
+      if (!cs.getTypeChecker().getLangOpts().isSwiftVersionAtLeast(5)) {
+        // Nothing to do for Swift 4 and earlier!
+        return simplifyExprType(expr);
+      }
+
+      Type subExprType = cs.getType(expr->getSubExpr());
+      Type targetType = simplifyType(subExprType);
+
+      // If the subexpression is not optional, wrap it in
+      // an InjectIntoOptionalExpr. Then use the type of the
+      // subexpression as the type of the 'try?' expr
+      bool subExprIsOptional = (bool) subExprType->getOptionalObjectType();
+
+      if (!subExprIsOptional) {
+        targetType = OptionalType::get(targetType);
+        auto subExpr = coerceToType(expr->getSubExpr(), targetType,
+                                    cs.getConstraintLocator(expr));
+        if (!subExpr) return nullptr;
+        expr->setSubExpr(subExpr);
+      }
+
+      cs.setType(expr, targetType);
+      return expr;
     }
 
     Expr *visitParenExpr(ParenExpr *expr) {

lib/Sema/CSDiagnostics.cpp

@@ -576,10 +576,22 @@ bool MissingOptionalUnwrapFailure::diagnoseAsError() {
   auto *tryExpr = dyn_cast<OptionalTryExpr>(unwrapped);
   if (!tryExpr)
     return diagnoseUnwrap(getConstraintSystem(), unwrapped, type);
-
-  emitDiagnostic(tryExpr->getTryLoc(), diag::missing_unwrap_optional_try, type)
-      .fixItReplace({tryExpr->getTryLoc(), tryExpr->getQuestionLoc()}, "try!");
-  return true;
+
+  bool isSwift5OrGreater = getTypeChecker().getLangOpts().isSwiftVersionAtLeast(5);
+  auto subExprType = getType(tryExpr->getSubExpr());
+  bool subExpressionIsOptional = (bool)subExprType->getOptionalObjectType();
+
+
+  if (isSwift5OrGreater && subExpressionIsOptional) {
+    // Using 'try!' won't change the type for a 'try?' with an optional sub-expr
+    // under Swift 5+, so just report that a missing unwrap can't be handled here.
+    return false;
+  }
+  else {
+    emitDiagnostic(tryExpr->getTryLoc(), diag::missing_unwrap_optional_try, type)
+    .fixItReplace({tryExpr->getTryLoc(), tryExpr->getQuestionLoc()}, "try!");
+    return true;
+  }
 }
 
 bool RValueTreatedAsLValueFailure::diagnoseAsError() {

lib/Sema/CSGen.cpp

@@ -1709,9 +1709,18 @@ namespace {
       if (!optTy)
         return Type();
 
-      CS.addConstraint(ConstraintKind::OptionalObject,
-                       optTy, CS.getType(expr->getSubExpr()),
-                       CS.getConstraintLocator(expr));
+      // Prior to Swift 5, 'try?' always adds an additional layer of optionality,
+      // even if the sub-expression was already optional.
+      if (CS.getTypeChecker().getLangOpts().isSwiftVersionAtLeast(5)) {
+        CS.addConstraint(ConstraintKind::Conversion,
+                         CS.getType(expr->getSubExpr()), optTy,
+                         CS.getConstraintLocator(expr));
+      }
+      else {
+        CS.addConstraint(ConstraintKind::OptionalObject,
+                         optTy, CS.getType(expr->getSubExpr()),
+                         CS.getConstraintLocator(expr));
+      }
       return optTy;
     }
 

lib/Sema/CSSimplify.cpp

@@ -2353,6 +2353,20 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
           forceUnwrapPossible = false;
         }
       }
+
+      if (auto optTryExpr =
+          dyn_cast_or_null<OptionalTryExpr>(locator.trySimplifyToExpr())) {
+        auto subExprType = getType(optTryExpr->getSubExpr());
+        bool isSwift5OrGreater = TC.getLangOpts().isSwiftVersionAtLeast(5);
+        if (isSwift5OrGreater && (bool)subExprType->getOptionalObjectType()) {
+          // For 'try?' expressions, a ForceOptional fix converts 'try?'
+          // to 'try!'. If the sub-expression is optional, then a force-unwrap
+          // won't change anything in Swift 5+ because 'try?' already avoids
+          // adding an additional layer of Optional there.
+          forceUnwrapPossible = false;
+        }
+      }
+
 
       if (forceUnwrapPossible) {
         conversionsOrFixes.push_back(