Rewrite empty without CE

[gusty]

A straight-forward implementation that doesn't rely on the CE

[gusty]

Why this?

CE empty taskSeq, call Current after MoveNextAsync returns false ... enumerator.Current |> should equal 0 // we return Unchecked.defaultof, which is Zero in the case of an integer

AFAIK that expectation is not part of the "contract".

[abelbraaksma]

This is a good addition, thanks! It'll likely create less overhead.

AFAIK that expectation is not part of the "contract".

Behavior of Current is undefined when accessed before or after the iteration. The behavior of other standard libs, like seq in F#, is to return Unchecked.defaultof<_>. We've chosen the same for consistency and "principle of least surprise".

The description of the test is wrong, probably copy/paste mistake. It should be something like:

``CE empty taskSeq, call Current after MoveNextAsync returns Unchecked-defaultof``

[abelbraaksma]

Thanks for this! Just a few nits below, otherwise LGTM. Not sure why the test report doesn't come up. Will rerun, see what that does.

[gusty]

Behavior of Current is undefined when accessed before or after the iteration. The behavior of other standard libs, like seq in F#, is to return Unchecked.defaultof<_>. We've chosen the same for consistency and "principle of least surprise".

> let e = Seq.empty<int>.GetEnumerator () ;;
val e: System.Collections.Generic.IEnumerator<int>

> e.Current ;;
System.InvalidOperationException: Enumeration has not started. Call MoveNext.
   at Microsoft.FSharp.Collections.IEnumerator.notStarted[a]() in D:\a\_work\1\s\src\FSharp.Core\seqcore.fs:line 20
   at Microsoft.FSharp.Collections.IEnumerator.EmptyEnumerator`1.System.Collections.Generic.IEnumerator<'T>.get_Current() in D:\a\_work\1\s\src\FSharp.Core\seqcore.fs:line 49
   at <StartupCode$FSI_0009>.$FSI_0009.main@() in C:\Users\gmpl2\AppData\Local\Temp\stdin:line 8
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
Stopped due to error 

Also, IMHO given an invalid operation, better fail fast. Tell the guy immediately he did something wrong, don't give him a no-sense value to make it fail later or even worst, not fail at all and have inconsistent data he'll maybe discover 1 year later.

PS: If we want to strictly stick to the seq model, we should give 2 different exceptions. I can certainly do that by adding a state variable, but IMHO the most important thing is to fail.

[abelbraaksma]

IMHO given an invalid operation, better fail fast.

Thers something to say for that. But I didn’t choose this approach by accident or personal preference. Let me check what the consensus is outside of that example (wouldn’t be the first time seq is inconsistent between operations).

[abelbraaksma]

@gusty I've updated the permissions for the CI report-tests jobs. This should (hopefully) fix the hanging jobs here (see: dorny/test-reporter#149 and #91). Can you rebase? (and yes, here we do love rebase for linear history, contrary to that other company ;) ).

(I could do it, but I'm afraid that as soon as I would, the permissions change and we'll get a false positive...)

[abelbraaksma]

Looks like the reporting is failing here for a different reason, it doesn't compile (does it compile locally?:

Error: D:\a\FSharp.Control.TaskSeq\FSharp.Control.TaskSeq\src\FSharp.Control.TaskSeq\TaskSeq.fs(18,59): error FS0039: The type 'ValueTask' does not define the field, constructor or member 'FromResult'.

[abelbraaksma]

I've fixed the compile error and did a bit of cleanup to clarify uses of ValueTask constructors. Not sure if my pushes will fix the Report tests to finally run, will see.

[abelbraaksma]

Following the guide on Dorny here, and after having it run at least once in main, it should now finally create the reports.

[abelbraaksma]

About whether or not raising an exception, I've answered that in #90 (specifically, this: #90 (comment)). Basically, both BCL and F# are not consistent here, and I would like to apply the principle of least surprise if at all possible. That is, taskSeq { do () } should behave the same as TaskSeq.empty. Note that seq { do () } and Seq.empty do not behave the same.

[abelbraaksma]

I've made a few changes to fix the compile error and to update the Utils with ValueTask members. Not entirely part of this PR, but badly needed for clarity.

Also updated the changelog.

Thanks for this change @gusty!

[abelbraaksma]

Thanks for this change, merged. edit: will be available in v0.3.0.