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

The problem was that in the by-address emission, we were calling
 `getAddressForInPlaceInitialization` twice, triggering the assert.

The first time in `emitExprInto` for the normal result case.

The second time to obtain the address again when generating the
catch block to inject a `.none` into that same address.

This patch does a light refactoring to more closely mirror
`visitOptionalEvaluationExpr`, which avoids calling the asserting method.

fixes rdar://80277465
This commit is contained in:
Kavon Farvardin
2023-09-18 15:41:43 -07:00
parent cda0fd4c84
commit 583f127de7
3 changed files with 132 additions and 19 deletions

View File

@@ -1184,18 +1184,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);
@@ -1208,8 +1217,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());
@@ -1219,8 +1227,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);
@@ -1238,10 +1249,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)
@@ -1268,8 +1277,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);
@@ -1287,9 +1295,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)

View File

@@ -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() ))

View File

@@ -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 }()
}