Merge pull request #68600 from kavon/optional-try-coercion-bug-v2

[SILGen] fix SR-14882: Assertion failed: (!concreteBuffer && "concrete buffer already formed?!")

Author: kavon

lib/SILGen/SILGenExpr.cpp

@@ -1210,18 +1210,27 @@ RValue RValueEmitter::visitOptionalTryExpr(OptionalTryExpr *E, SGFContext C) {
 
   // Form the optional using address operations if the type is address-only or
   // if we already have an address to use.
-  bool isByAddress = usingProvidedContext || optTL.isAddressOnly();
+  bool isByAddress = ((usingProvidedContext || optTL.isAddressOnly()) &&
+      SGF.silConv.useLoweredAddresses());
 
   std::unique_ptr<TemporaryInitialization> optTemp;
-  if (!usingProvidedContext && isByAddress) {
+  if (!isByAddress) {
+    // If the caller produced a context for us, but we're not going
+    // to use it, make sure we don't.
+    optInit = nullptr;
+  } else if (!usingProvidedContext) {
     // Allocate the temporary for the Optional<T> if we didn't get one from the
-    // context.
+    // context.  This needs to happen outside of the cleanups scope we're about
+    // to push.
     optTemp = SGF.emitTemporary(E, optTL);
     optInit = optTemp.get();
-  } else if (!usingProvidedContext) {
-    // If the caller produced a context for us, but we can't use it, then don't.
-    optInit = nullptr;
   }
+  assert(isByAddress == (optInit != nullptr));
+
+  // Acquire the address to emit into outside of the cleanups scope.
+  SILValue optAddr;
+  if (isByAddress)
+    optAddr = optInit->getAddressForInPlaceInitialization(SGF, E);
 
   FullExpr localCleanups(SGF.Cleanups, E);
 
@@ -1234,8 +1243,7 @@ RValue RValueEmitter::visitOptionalTryExpr(OptionalTryExpr *E, SGFContext C) {
   SILValue branchArg;
   if (shouldWrapInOptional) {
     if (isByAddress) {
-      assert(optInit);
-      SILValue optAddr = optInit->getAddressForInPlaceInitialization(SGF, E);
+      assert(optAddr);
       SGF.emitInjectOptionalValueInto(E, E->getSubExpr(), optAddr, optTL);
     } else {
       ManagedValue subExprValue = SGF.emitRValueAsSingleValue(E->getSubExpr());
@@ -1245,8 +1253,11 @@ RValue RValueEmitter::visitOptionalTryExpr(OptionalTryExpr *E, SGFContext C) {
   }
   else {
     if (isByAddress) {
-      assert(optInit);
-      SGF.emitExprInto(E->getSubExpr(), optInit);
+      assert(optAddr);
+      // We've already computed the address where we want the result.
+      KnownAddressInitialization normalInit(optAddr);
+      SGF.emitExprInto(E->getSubExpr(), &normalInit);
+      normalInit.finishInitialization(SGF);
     } else {
       ManagedValue subExprValue = SGF.emitRValueAsSingleValue(E->getSubExpr());
       branchArg = subExprValue.forward(SGF);
@@ -1264,10 +1275,8 @@ RValue RValueEmitter::visitOptionalTryExpr(OptionalTryExpr *E, SGFContext C) {
     if (!isByAddress)
       return RValue(SGF, E,
                     SGF.emitManagedRValueWithCleanup(branchArg, optTL));
-    
-    if (shouldWrapInOptional) {
-      optInit->finishInitialization(SGF);
-    }
+
+    optInit->finishInitialization(SGF);
 
     // If we emitted into the provided context, we're done.
     if (usingProvidedContext)
@@ -1294,8 +1303,7 @@ RValue RValueEmitter::visitOptionalTryExpr(OptionalTryExpr *E, SGFContext C) {
   catchCleanups.pop();
 
   if (isByAddress) {
-    SGF.emitInjectOptionalNothingInto(E,
-                    optInit->getAddressForInPlaceInitialization(SGF, E), optTL);
+    SGF.emitInjectOptionalNothingInto(E, optAddr, optTL);
     SGF.B.createBranch(E, contBB);
   } else {
     auto branchArg = SGF.getOptionalNoneValue(E, optTL);
@@ -1313,9 +1321,7 @@ RValue RValueEmitter::visitOptionalTryExpr(OptionalTryExpr *E, SGFContext C) {
     return RValue(SGF, E, SGF.emitManagedRValueWithCleanup(arg, optTL));
   }
 
-  if (shouldWrapInOptional) {
-    optInit->finishInitialization(SGF);
-  }
+  optInit->finishInitialization(SGF);
 
   // If we emitted into the provided context, we're done.
   if (usingProvidedContext)

test/Runtime/optional_try.swift

@@ -0,0 +1,54 @@
+// RUN: %target-run-simple-swift -sil-verify-all | %FileCheck %s
+// REQUIRES: executable_test
+
+enum Bad: Error {
+    case err
+    case custom(String)
+}
+
+func erase<T>(_ val: T) -> Any {
+    return val as Any
+}
+
+class Klass {}
+
+typealias MaybeString = Result<String, Error>
+typealias MaybeKlass = Result<Klass, Error>
+typealias MaybeInt = Result<Int, Error>
+typealias MaybeNumbers = Result<[Int], Error>
+
+////////
+// NOTE: Do _not_ heed the warnings about implicit coercions to Any.
+//       That's an important part of this test's coverage!
+////////
+// -- throwing --
+// CHECK: nil
+print( try? MaybeString.failure(Bad.err).get() )
+
+// CHECK: nil
+print( try? MaybeKlass.failure(Bad.custom("doggo")).get() )
+
+// CHECK: nil
+print( try? MaybeInt.failure(Bad.err).get() )
+
+// CHECK: nil
+print( try? MaybeNumbers.failure(Bad.err).get() )
+
+// CHECK: nil
+print(erase( try? MaybeNumbers.failure(Bad.err).get() ))
+
+// -- normal --
+// CHECK: Optional("catto")
+print( try? MaybeString.success("catto").get() )
+
+// CHECK: Optional(main.Klass)
+print( try? MaybeKlass.success(Klass()).get() )
+
+// CHECK: Optional(3)
+print( try? MaybeInt.success(3).get() )
+
+// CHECK: Optional([4, 8, 15, 16, 23, 42])
+print( try? MaybeNumbers.success([4, 8, 15, 16, 23, 42]).get() )
+
+// CHECK: Optional([0, 1, 1, 2, 3])
+print(erase( try? MaybeNumbers.success([0, 1, 1, 2, 3]).get() ))

test/SILGen/optional.swift

@@ -161,3 +161,56 @@ func implicit_iuo_unwrap_sourceLocation(_ value: Int!) {
   use_unwrapped(value)
 #sourceLocation() // reset
 }
+
+
+// CHECK-LABEL:  sil hidden [ossa] @$s8optional0A20_to_existential_castyyF : $@convention(thin) () -> () {
+// CHECK:  bb0:
+// CHECK:    [[MEM:%.*]] = alloc_stack $Any
+// CHECK:    [[ADDR:%.*]] = init_existential_addr [[MEM]] : $*Any, $Optional<Int>
+// CHECK:    [[PAYLOAD_ADDR:%.*]] = init_enum_data_addr [[ADDR]] : $*Optional<Int>, #Optional.some!enumelt
+// CHECK:    [[CLOSURE:%.*]] = function_ref @$s8optional0A20_to_existential_castyyFSiyKXEfU_ : $@convention(thin) () -> (Int, @error any Error)
+// CHECK:    try_apply [[CLOSURE]]() : $@convention(thin) () -> (Int, @error any Error), normal bb1, error bb3
+
+// CHECK:  bb1([[RESULT:%.*]] : $Int):
+// CHECK:    store [[RESULT]] to [trivial] [[PAYLOAD_ADDR]] : $*Int
+// CHECK:    inject_enum_addr [[ADDR]] : $*Optional<Int>, #Optional.some!enumelt
+// CHECK:    br bb2
+
+// CHECK:      bb2:
+// CHECK-NEXT:   destroy_addr [[MEM]] : $*Any
+// CHECK-NEXT:   dealloc_stack [[MEM]] : $*Any
+// CHECK:        return
+
+// CHECK:  bb3([[ERR:%.*]] : @owned $any Error):
+// CHECK:    destroy_value [[ERR]] : $any Error
+// CHECK:    inject_enum_addr [[ADDR]] : $*Optional<Int>, #Optional.none!enumelt
+// CHECK:    br bb2
+// CHECK:  }
+func optional_to_existential_cast() {
+  let _: Any = try? {() throws in 3 }()
+}
+
+// CHECK-LABEL: sil hidden [ossa] @$s8optional0A29_to_existential_cast_RETURNEDypyF : $@convention(thin) () -> @out Any {
+// CHECK:  bb0([[MEM:%.*]] : $*Any):
+// CHECK:    [[ADDR:%.*]] = init_existential_addr [[MEM]] : $*Any, $Optional<Int>
+// CHECK:    [[PAYLOAD_ADDR:%.*]] = init_enum_data_addr [[ADDR]] : $*Optional<Int>, #Optional.some!enumelt
+// CHECK:    [[CLOSURE:%.*]] = function_ref @$s8optional0A29_to_existential_cast_RETURNEDypyFSiyKXEfU_ : $@convention(thin) () -> (Int, @error any Error)
+// CHECK:    try_apply [[CLOSURE]]() : $@convention(thin) () -> (Int, @error any Error), normal bb1, error bb3
+
+// CHECK:  bb1([[RESULT:%.*]] : $Int):
+// CHECK:    store [[RESULT]] to [trivial] [[PAYLOAD_ADDR]] : $*Int
+// CHECK:    inject_enum_addr [[ADDR]] : $*Optional<Int>, #Optional.some!enumelt
+// CHECK:    br bb2
+
+// CHECK:      bb2:
+// CHECK-NEXT:  = tuple ()
+// CHECK-NEXT:        return
+
+// CHECK:  bb3([[ERR:%.*]] : @owned $any Error):
+// CHECK:    destroy_value [[ERR]] : $any Error
+// CHECK:    inject_enum_addr [[ADDR]] : $*Optional<Int>, #Optional.none!enumelt
+// CHECK:    br bb2
+// CHECK:  }
+func optional_to_existential_cast_RETURNED() -> Any {
+  return try? {() throws in 3 }()
+}