fsprojects / FSharp.Control.TaskSeq

A computation expression and module for seamless working with IAsyncEnumerable<'T> as if it is just another sequence
MIT License
91 stars 7 forks source link

Unintended object inference #256

Closed JohSand closed 2 days ago

JohSand commented 2 months ago

Binding task-likes in the taskSeq-builder will trigger the FS3559-warning. This seems to be due to the existence of the 'TOverall parameter here:

        [<NoEagerConstraintApplication>]
        member inline Bind< ^TaskLike, 'T, 'U, ^Awaiter, 'TOverall> :
            task: ^TaskLike * continuation: ('T -> ResumableTSC<'U>) -> ResumableTSC<'U>
                when ^TaskLike: (member GetAwaiter: unit -> ^Awaiter)
                and ^Awaiter :> ICriticalNotifyCompletion
                and ^Awaiter: (member get_IsCompleted: unit -> bool)
                and ^Awaiter: (member GetResult: unit -> 'T)

This parameter cannot be inferred, since it is not used in the method, and is probably only present due to copy-paste.

abelbraaksma commented 2 months ago

Hi @JohSand, thanks for reporting this.

and is probably only present due to copy-paste.

~The 'TOverall cannot be removed here (at least, I remember it wasn't removable, but I may be wrong, this part was done 2 years ago and I don't recall why it's there). Do you happen to have a small repo for this that I can add as test?~

EDIT: I was wrong. Looks like it works just fine after removing it. @JohSand If you can test the linked PR to verify it removes the warning, please do (and if you have a repo, even better!).

To be fair, when designing this, I didn't expect task-like types to be used very often and I don't think I have proper coverage in the tests, which ought to have uncovered this issue.

I'm glad there are use-cases "in the wild" :) and I hope there's a straightforward fix :).

JohSand commented 2 months ago

Hi. For a repro, the easiest way is to bind Task.Yield(), which does in fact not return a Task, but a YieldAwatable.

So with FS3559, I can repro it with code like

    taskSeq {
        do! Task.Yield()
        yield 1
    }

I can also confirm that remove 'TOverall removes the warning.

abelbraaksma commented 2 months ago

Awesome. I'll add a few tests tomorrow and merge the PR. Thank you very much for reporting this!