Fix two bugs related to tasks by dsyme · Pull Request #12195 · dotnet/fsharp · G...

dsyme commented 5 days ago •

This fixes two problems reported for tasks

#12189 is bad codegen for generic tasks inside a generic class. It's fixed by more careful coding in IlxGen.fs

#12188 is a more subtle fix. The bug stems from the fact that our processing of SRTP constraints for generic task code is unfortunately hitting known problems with processing SRTP constraints. This only happens in situations where task { .. } code is underspecified w.r.t. the types involved in binding, e.g.

let myFunction f =
    task {
        do! f ()
        return ()
    }

This code is not wrong - however its ambiguity can currently cause later inference problems. In particular, the return type of f is not known to be a task, nor is it known to be any of the other available binding types (Task, "Task like" or Async), leaving the type inference SRTP constraints unsolved, and possibly resolved later in the file. This ambiguity can cause later problems, described below.

The workaround is always to make the types being bound immediately clear:

let myFunction (f: unit-> Task<_>) =
    task {
        do! f ()
        return ()
    }

The underlying problem goes right back to the mis-inclusion of #1650 into F#. In short #1650 got (very rashly) included in F# 4.0, we backed it out, but people had already started to take dependencies on it and reinstated it for stability. #1650 effectively introduced a new rule into F# type checking along the lines "If adding a constraint means a type inference problem gives rise to an unresolved SRTP constraint, then continue without further processing the constraint". The unfortunate effect of this has always been that, in some situations, constraints are not fully applied to F# type checking - constraint solving is "aborted" when SRTP constraints can't be solved. Normally this isn't a problem because, for various reasons, the constraints are re-solved later. However that doesn't always happen, and indeed #12188 is one such case, and the end result is bad code generation.

Now, the "obvious" fix is to always fully-applying the constraints and remove #1650. However this changes the order of type inference which can affect name resolution and other type checking results.

This systematically fixes this problem without changing type inference order, by

1. saving ("postponing") the constraints when #1650 kicks in
2. applying them at the end of inference, before defaults are applied.

This fixes the immediate problem for #12188

• Assess its impact (all tests pass)
• Add testing for #12188 and #12189
• Get it green