Possible fix for typed-throws re-abstraction thunk crash

the compiler currently crashes on code like this:

func nothrow() {}

func noThrowAsThrows<E: Error>() -> () throws(E) -> Void { nothrow }

let _: () -> Void = noThrowAsThrows() // 💥

if i've understood the issue correctly, the problem is that a re-abstraction thunk is needed to convert the non-throwing closure to one with a typed-throws calling convention. in this case, to convert from () -> Void to () throws(E) -> Void, an 'original-to-substituted' thunk is required.

currently, it seems only the other direction of thunk can successfully be produced in these cases – the 'substituted-to-original' variant. when constructing the original-to-substituted thunk, it ends up trying to emit 'rethrowing' logic, and an internal invariant fails when attempting to determine the error type to use for the conversion (the thunk has no error type, since it's converting to a non-throwing function).

if we try to do the simplest thing, and modify the existing code to resolve the error type to Never and sidestep the assertion, the generated thunk looks like this:

// thunk for @escaping @callee_guaranteed () -> (@error @out Never)
sil shared [transparent] [serialized] [reabstraction_thunk] [ossa] @$ss5NeverOIegzr_Ieg_TR : $@convention(thin) (@guaranteed @callee_guaranteed () -> @error_indirect Never) -> () {
// %0                                             // user: %2
bb0(%0 : @guaranteed $@callee_guaranteed () -> @error_indirect Never):
  %1 = alloc_stack $Never                         // users: %5, %8, %7, %2
  try_apply %0(%1) : $@callee_guaranteed () -> @error_indirect Never, normal bb1, error bb2 // id: %2

bb1(%3 : $()):                                    // Preds: bb0
  %4 = tuple ()                                   // user: %6
  dealloc_stack %1                                // id: %5
  return %4                                       // id: %6

bb2:                                              // Preds: bb0
  %7 = load [trivial] %1                          // user: %9
  dealloc_stack %1                                // id: %8
  throw %7                                        // id: %9
} // end sil function '$ss5NeverOIegzr_Ieg_TR'

however, this is invalid because in the error branch it throws, but the thunk itself isn't throwing. i've implemented a possible solution to that here [WIP]: test fix for throws(Never) thunk crash by jamieQ · Pull Request #86619 · swiftlang/swift · GitHub, in which an unreachable is emitted into the error branch, and that has so far seemed to get things working. that change produces the following re-abstraction thunk:

// thunk for @escaping @callee_guaranteed () -> (@error @out Never)
sil shared [transparent] [reabstraction_thunk] @$ss5NeverOIegzr_Ieg_TR : $@convention(thin) (@guaranteed @callee_guaranteed () -> @error_indirect Never) -> () {
// %0                                             // user: %2
bb0(%0 : $@callee_guaranteed () -> @error_indirect Never):
  %1 = alloc_stack $Never                         // users: %5, %2
  try_apply %0(%1) : $@callee_guaranteed () -> @error_indirect Never, normal bb1, error bb2 // id: %2

bb1(%3 : $()):                                    // Preds: bb0
  %4 = tuple ()                                   // user: %6
  dealloc_stack %1                                // id: %5
  return %4                                       // id: %6

bb2:                                              // Preds: bb0
  unreachable                                     // id: %7
} // end sil function '$ss5NeverOIegzr_Ieg_TR'

so, my question is – is this a reasonable approach to address this problem? the codegen seems a bit odd because it's still allocating space for a Never error result. but perhaps there's no way around that at this stage of lowering, and it can be optimized away later? if this isn't heading down the right path, what would be a better solution?


and if anyone cares to indulge me more on the history/motivation here... it seems these thunks actually used to not be produced at all – that logic was introduced here: [SILGen] Create thunks for non-throwing -> indirect error result conversions by kateinoigakukun · Pull Request #73162 · swiftlang/swift · GitHub. however, i don't totally follow the motivation as laid out in the PR description. was the code 'working' before that change, or was this sort of thing always necessary and without the thunking there would be runtime issues?

3 Likes

I also thought the alloc_stack Never was surprising, and I wonder if its completely sound. However, a direct call to such a function generates the same result:

func noThrowAsThrows<E: Error>(_: E.Type) throws(E) -> () {}

noThrowAsThrows(Never.self)
  %2 = metatype $@thin Never.Type
  %3 = metatype $@thick Never.Type                // user: %6
  // function_ref noThrowAsThrows<A>(_:)
  %4 = function_ref @$s5error15noThrowAsThrowsyyxmxYKs5ErrorRzlF : $@convention(thin) <τ_0_0 where τ_0_0 : Error> (@thick τ_0_0.Type) -> @error_indirect τ_0_0 // user: %6
  %5 = alloc_stack $Never                         // users: %8, %6
  try_apply %4<Never>(%5, %3) : $@convention(thin) <τ_0_0 where τ_0_0 : Error> (@thick τ_0_0.Type) -> @error_indirect τ_0_0, normal bb1, error bb2 // id: %6

bb1(%7 : $()):                                    // Preds: bb0
  dealloc_stack %5                                // id: %8
  %9 = integer_literal $Builtin.Int32, 0          // user: %10
  %10 = struct $Int32 (%9)                        // user: %11
  return %10                                      // id: %11

bb2:                                              // Preds: bb0
  unreachable                                     // id: %12

With -O, this becomes:

  %4 = alloc_stack $Never                         // users: %5, %6
  %5 = apply [nothrow] %3<Never>(%4, %2) : $@convention(thin) <τ_0_0 where τ_0_0 : Error> (@thick τ_0_0.Type) -> @error_indirect τ_0_0

Perhaps it could just pass an undef instead of %4 as the argument to the apply.

2 Likes

alloc_stack $Never by itself should be fine, you just can never actually initialize or load from the "memory" that notionally gets allocated, though you can pass the address along to other functions (which can, transitively, never actually load or store anything there).

3 Likes

that all makes sense – thank you both. here's a proposed solution, which has a smattering of tests and has made it through CI: [SILGen]: fix thunk emission for nothrow to throws(Never) by jamieQ · Pull Request #86619 · swiftlang/swift · GitHub. would appreciate feedback if/when you have the time.