fsharp icon indicating copy to clipboard operation
fsharp copied to clipboard

Using computation builder `TryWith` with a non-`Exception` parameter generates invalid IL

Open IS4Code opened this issue 2 months ago • 8 comments

When a computation builder defines a TryWith method whose exception handler does not actually take an Exception as its parameter, an invalid CIL is produced from such a try/with construction.

Repro steps

type ResultBuilder() =
  member _.Delay(f : _ -> Result<_, _>) = f
  member _.Run(f : _ -> Result<_, _>) = f()

  member _.Zero() = Ok()
  member _.Return(value) = Ok value
  member _.ReturnFrom(result : Result<_, _>) = result
  member _.Bind(result : Result<_, _>, f) = Result.bind f result
  member _.Combine(result : Result<_, _>, f) = Result.bind f result
  
  member _.TryWith(f : unit -> _, fail : _ -> _) =
    let result = f()
    match result with
    | Ok _ -> result
    | Error _ -> fail result

let result = ResultBuilder()

let test() =
 result { 
   try 
    do! Error "test"
   with
   | Error msg -> System.Console.WriteLine msg
 }

let _ = test()

Expected behavior

The code should either fail to compile, or compile to a program that runs and behaves as expected, i.e. prints "test".

Actual behavior

The code compiles but fails at runtime with:

System.InvalidProgramException: Common Language Runtime detected an invalid program.

This is caused by the construction of ExceptionDispatchInfo in case the "exception" is not matched:

ldarg.1 // valuetype [FSharp.Core]Microsoft.FSharp.Core.FSharpResult`2<class [FSharp.Core]Microsoft.FSharp.Core.Unit, string>
call class [System.Runtime]System.Runtime.ExceptionServices.ExceptionDispatchInfo [System.Runtime]System.Runtime.ExceptionServices.ExceptionDispatchInfo::Capture(class [System.Runtime]System.Exception)
callvirt instance void [System.Runtime]System.Runtime.ExceptionServices.ExceptionDispatchInfo::Throw()

This attempts to use a Result argument instead of Exception, resulting in an invalid program.

Known workarounds

Using actual exceptions for this purpose.

Related information

Environment: .NET 9.0.303, sharplab.io

IS4Code avatar Nov 09 '25 12:11 IS4Code

I see a few obvious ways out of this issue:

  • Disallow it, i.e. try/with in a computation must always be constrained to Exception. This is better than the current situation, but it is unfortunate because this could be a cool way to have "custom exceptions".
  • Fix the "rethrow" so that it doesn't involve ExceptionDispatchInfo if the parameter is statically known not to be a subclass of Exception. Then there are a few options of what happens next:
    • Do not wrap it in any exception and just throw it as an object. This is permitted by CIL, but then it could most likely run into #18374.
    • Throw a new special exception that specifically signalizes the exception handler failed to match the "exception". This seems reasonable, given that TryWith should already be prepared for that situation, however this brings the runtime overhead of exceptions into a mechanism that "shouldn't" depend on them.
  • Disallow situations that require rethrowing, i.e. if the parameter is not a subclass of Exception, the patterns after with must be total. This would break the example code, but it could be modified to unwrap the error and take that.
  • Do the previous both, but only as a warning, i.e. if the patterns are not total, warn that an exception will be thrown. This seems like the best option, given the current API.
  • Fix the API. Either add a new Reraise method that is invoked in this situation, or permit exception handlers that return _ option or _ voption to indicate match failure, and let the "rethrow" logic be done in TryWith.

With the last option, the code could be rewritten as:

member _.TryWith(f : unit -> _, [<MayNotMatchArgument>]fail : _ -> voption _) =
 let result = f()
 match result with
 | Ok _ -> result
 | Error _ ->
   match fail result with
   | ValueSome handled -> handled
   | ValueNone -> result

That would be a major win for F# against "exceptionless" languages.

IS4Code avatar Nov 09 '25 13:11 IS4Code

I vote for making type checking stricter and disallowing anything that is not Exception.

I do fail to see the value of using this with a non wxception argument because non-exceptional error cases are already perfectly representable with result typeband it's not clear how would compiler even jump to the non exceptional catch handler.

Also I'm not sure it is reasonable to expand try/with compexpr construct without expanding the non compexpr try-with and proving the worth of it is a huge design exploration.

En3Tho avatar Nov 09 '25 13:11 En3Tho

That's kinda sophistry. Sure, you can't have a normal try/with with non-exceptions (ignoring CIL shenanigans) because .NET supports only one mechanism of exceptional control flow, but that's what computation expressions are for. They can simulate gotos, early returns, iterate uniterable things, dispose undisposable things. Catching non-exception values is not so far-fetched, and it is extremely clear how that should work: just call TryWith and figure out how to implement "reraise". No point trying to figure out how should that work with non-computation try/with because 99 % of what computation expressions can do doesn't work normally anyway.

Even the code above could work as it is with these issues solved. It could even work right now (with flaws) if you have fail : 'T -> _ when 'T :> Exception and check the type of 'T at runtime, then synthesize the exception.

IS4Code avatar Nov 09 '25 17:11 IS4Code

This is a bug, the documentation says the handler's input must be exn:

|`TryWith`|`Delayed<'T> * (exn -> M<'T>) -> M<'T>`|Called for `try...with` expressions in computation expressions.|

All the other options would need to go via a suggestions and have sufficient motivation. Exception-free programing , also possibly comined with a non-.NET target via Fable, might be a curious exploration area.

T-Gro avatar Nov 10 '25 10:11 T-Gro

Just to be clear, would #exn or anything that constrains it to a type derived from Exception be okay? I tried to look it up in the specification but couldn't find an argument against it. That case does not result in invalid CIL and has some uses (getting strongly-typed exception information, checking incorrect exception types etc.) so prohibiting that would be a breaking change.

Also I would be cautious taking the documentation at face value: for example it also sticks to seq<'T> in For, even though that is neither accurate (query, taskSeq, and plenty more support other types of collections) nor precise (normal for supports duck-typing too).

IS4Code avatar Nov 10 '25 11:11 IS4Code

#exn as a check will be okay. It just has to be done careful w.r.t to buiders which just delegate to a call, that style of coding a CE must remain working and not negatively affect type inference (is doable for sure, and most likely it will work out thanks to transitive type inference flowing from a non-CE try-with somewhere that already typechecks the matched value to be #exn )

https://github.com/fs-fio/fio/blob/e46a1211cd661c464f4fca7c22ab1a13fca312ef/src/FSharp.FIO/DSL/CE.fs#L126

https://github.com/fsprojects/FSharpPlus/blob/e280cfe5f8f419323147f458dabc10243f04f792/src/FSharpPlus/Builders.fs#L81

https://github.com/haf/expecto/blob/65cf1583770e13980a592b4511b611347e1a212f/Expecto/Expecto.fs#L212

T-Gro avatar Nov 10 '25 12:11 T-Gro

@T-Gro I'm not sure how a type constrained to exception would work here. Will it work similarly to exception handler with "when" clause baked in? Or does it basicaly always get resolved as as "Exception"

En3Tho avatar Nov 10 '25 18:11 En3Tho

One way to do it would be to raise an error if this isn't met. Other would be like you say - inferring it automatically for the member just like happens for then try - with clause.

T-Gro avatar Nov 11 '25 08:11 T-Gro