Uh oh!
There was an error while loading. Please reload this page.
Fix two bugs related to tasks - #12195
Conversation
…nd put it in the constraint solver context instead
…nd put it in the constraint solver context instead
…nd put it in the constraint solver context instead
…nd put it in the constraint solver context instead
Happypig375
commented
Sep 28, 2021
@gusty for original submitter |
| trackErrors { | ||
| do! errors | ||
| let cxsln = Option.map (fun traitInfo -> (traitInfo, MemberConstraintSolutionOfMethInfo csenv.SolverState m calledMeth.Method calledMeth.CalledTyArgs)) cx | ||
| let cxsln = AssumeMethodSolvesTrait csenv cx m trace calledMeth |
@TIHan@vzarytovskii I pushed one additional update to this to make sure we pop the actions if there is an undo-trace active - will merge when green |
vzarytovskii
commented
Sep 29, 2021
dsyme
commented
Sep 29, 2021
@vzarytovskii yes absolutely. I'll merge now and set up the back port tomorrow |
* fix srtp processing related to tasks * fix 12189 - bad codegen for tasks. Also eliminate 'trace' parameter and put it in the constraint solver context instead * fix 12189 - bad codegen for tasks. Also eliminate 'trace' parameter and put it in the constraint solver context instead * fix 12189 - bad codegen for tasks. Also eliminate 'trace' parameter and put it in the constraint solver context instead * fix 12189 - bad codegen for tasks. Also eliminate 'trace' parameter and put it in the constraint solver context instead * cleanup and fix method arg lambda propagation rule * fix error messages * fix error messages * fix error messages * fix error messages * simplify diff * simplify diff * reduce diff and fix errors * reduce diff and fix errors
* fix srtp processing related to tasks * fix 12189 - bad codegen for tasks. Also eliminate 'trace' parameter and put it in the constraint solver context instead * fix 12189 - bad codegen for tasks. Also eliminate 'trace' parameter and put it in the constraint solver context instead * fix 12189 - bad codegen for tasks. Also eliminate 'trace' parameter and put it in the constraint solver context instead * fix 12189 - bad codegen for tasks. Also eliminate 'trace' parameter and put it in the constraint solver context instead * cleanup and fix method arg lambda propagation rule * fix error messages * fix error messages * fix error messages * fix error messages * simplify diff * simplify diff * reduce diff and fix errors * reduce diff and fix errors
JonCanning
commented
Oct 6, 2021
@JonCanning If you have the given type annotation, the type is Task, as you'd expect. After this fix, you can type-annotate the other bindable types ( letmyFunction(f:unit ->Async<_>)=task{return! f ()}If there is no strongly known type at the resolution of >letmyFunction f =-task{-return! f ()-};;valmyFunction:f:(unit ->#Task<'b>) ->Task<'b> |
In my example it's adding the |
@JonCanning Ok. I checked all the following on latest bits after this fix and the inference now works as expected openSystem.Threading.TasksletmyFunction value (f:unit ->Async<_>)=task{if value thendo! Task.Delay 1000return! f ()}letmyFunction value (f:unit ->Task<_>)=task{if value thendo! Task.Delay 1000return! f ()}letmyFunction value f =task{if value thendo! Task.Delay 1000return! f ()}give |

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.This code is not wrong - however its ambiguity can currently cause later inference problems. In particular, the return type of
fis 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:
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
This fixes the immediate problem for #12188