Comments (17)
So if I understand correctly we should just remove the Return
from the builder in this repo (and accept the breaking change if people are using return
explicitly in asyncSeq
)
from fsharp.control.asyncseq.
As a workaround to stop the pain now, I've added this to our project to define a version of the CE builder without the Return:
namespace FSharp
open FSharp.Control
/// Computation builder that allows creating of asynchronous sequences using the 'asyncSeq { ... }' syntax
/// This is a "fixed" version without the value-ignoring Return
type FixedAsyncSeqBuilder() =
let builder = AsyncSeq.AsyncSeqBuilder()
member __.Bind (source, body) = builder.Bind (source, body)
member __.Combine (seq1, seq2) = builder.Combine (seq1, seq2)
member __.Delay f = builder.Delay f
member __.For (source: 'a seq, action) = builder.For (source, action)
member __.For (source: 'a AsyncSeq, action) = builder.For (source, action)
member __.TryFinally (body, comp) = builder.TryFinally (body, comp)
member __.TryWith (body, handler) = builder.TryWith (body, handler)
member __.Using (resource, binder) = builder.Using (resource, binder)
member __.While (guard, body) = builder.While (guard, body)
member __.Yield value = builder.Yield value
member __.YieldFrom source = builder.YieldFrom source
member __.Zero () = builder.Zero ()
module Control =
// shadow the builder from FSharp.Control
let asyncSeq = FixedAsyncSeqBuilder()
from fsharp.control.asyncseq.
I'll try do an updated summary because i got same issue ( #92 ), it's really error prone :D more so migrating from v1 to v2
So the compiler feature from @mexx was released in F# 4.1
NOTE
- the
FSharp.Core
version doesnt matter (it's ok we continue to target >= v3), it matter just the compiler version. Right? /cc @mexx @dsyme - Until support for compiler builtin in VS 2015 is dropped by
asyncseq
, it's not possibile to use the new feature.
Q: Can be useful to do it just for netstandard2.0
? that will add an #ifdef NETSTANDARD2_0
to remove the Return
from asyncsec builder (if i understand right the change required by the feature).
- PRO fix the issue for
netstandard
, because it require a F# 4.1+ compiler so we can expect to have that feature - PRO doesnt change anything for users targeting
net
only (no api surface changes), for older or newer compiler - CONS1 if a code multitarget both
net
andnetstandard
(or addnetstandard
after a migration), any usage ofreturn
will fail to compile (well, will be ok innet
, but fails innetstandard
). - CONS2 this change the current api surface for
netstandard2.0
, so it's anyway a breaking change -> fsharp.control.asyncseq v3**. but user with v2 can meanwhile start change the code to remove it, so override to v3 will work anyway.
I think the CONS1 is instead a nice to have.
Will break some compilation, but usage of return
directly is 99% a bug afaik.
I cannot think why someone should use it directly, the desugar of while
is done by compiler.
So doing that will surface a bug in the user implementation.
any code like this
let a = asyncSeq {
yield 1
return 2 // this compile, but 2 is ignored, so a bug without error
}
will need to be changed to
let a = asyncSeq {
yield 1
}
who does the same thing.
@eulerfx @mexx @dsyme make sense?
from fsharp.control.asyncseq.
Yes, IIRC this is an F# language limitation where it is problematic to express that a method takes a single parameter of type unit
, rather than no parameters. It's almost never needed but the computation expression translation uses both foo.Return(())
and foo.Return()
so we need a method that can accept either. If you can find an alternative workaround it would be much appreciated.
from fsharp.control.asyncseq.
@dsyme I've tried a patched version of compiler and could successfully remove the Return
method from the builder.
As stated in the initial comment, the call Zero()
instead of the Return()
allows constructs like
asyncSeq {
yield 1
do! Async.Sleep 2000
yield 30
}
to be processed without the need for Return
method.
Should I create a PR with the compiler change in visualfsharp for further discussion?
from fsharp.control.asyncseq.
Hi @maxx
We wouldn't change Return to Zero - that would be a breaking change. We might however remove the limitation that Return gets called with both ()
(zero) and (())
(one) arguments, which if IIRC is the cause of the problem.
However even that change might not meeting our no-breaking-change policy, and we might have to resort to just emitting a warning of some kind.
from fsharp.control.asyncseq.
I think my concern is not clear. In the current asyncSeq
one can use return whatever
and the value of whatever
expression is not processed any further by the asyncSeq
. Furthermore the asyncSeq { return [1] }
is an empty sequence. This is not intuitive and do not correspond to the behavior of other computation expressions. So IMHO the root cause is the presence of return
.
Don, you are right about the breaking change if we simply replace the return ()
with zero
. But at the time we inject the behavior we know about what methods are present on the builder and could inject Zero
when there is no Return
. This would allow us to write AsyncSeqBuilder
without having the Return
method on it and the asyncSeq { return whatever }
would become illegal at compile time.
from fsharp.control.asyncseq.
@dsyme I've updated my patch to fallback to Zero
if Return
is not present, you can find it here.
from fsharp.control.asyncseq.
That approach looks good! Please add an fslang.uservoice.com entry for it - I'll mark it as approved for some future release - and submit the PR to Microsoft/visualfsharp.
It will take a long while before libraries like AsyncSeq can rely on this language change though, since it basically currently targets FSharp.Core 4.3.0.0 and supports use by F# 3.0+
from fsharp.control.asyncseq.
from fsharp.control.asyncseq.
@dsyme
While implementing the tests for the PR on visualfsharp repo, I could define the implementation for return
as Return (()) = empty
which will be picked correctly for the unit
parameter, and therefore allow only unit
valued expressions to be used with return
keyword. It doesn't work when you define Return() = empty
. Is it intended to work this way?
It's very weird, actually the distinguishing problem only arises from the usage of signature file.
Only in the signature file I didn't found a way to define that there should be one parameter of type unit
. Is there a way to define such signature in the .fsi
file?
from fsharp.control.asyncseq.
@mexx following up on this, is there still some action to take on this library or has focused shifted elsewhere?
from fsharp.control.asyncseq.
The next version of F# compiler will allow to use Zero
without the need to provide Return
.
With this version this library could get rid of the Return
method on the builder. However this would be a breaking change for users of the library.
from fsharp.control.asyncseq.
I was extremely confused by this as well. My use case was yielding a single value in the sequence given an Async<T>
. I used return, which type checked, so I was expecting the async seq to have this one value produced by the async expression, but got nothing. The crux of this issue is that the builder's return accepts any parameter and ignores it.
The adjustment to the compiler is nice, since I ran into that do!
issue with my own CE as well - but I'd like to suggest a simple workaround in the meantime that does not cause this confusion: restrict Return's parameter type to unit. This allows you to continue to use do!
and return ()
but not return 1
or any other arbitrary value. This would cause code written against this library to fail to compile if the code returns non-unit values, but this can only be a good thing since such code was probably written under the misconception that something was being done with the value.
from fsharp.control.asyncseq.
I tried to create a pull request for the above, but encountered a problem with specifying the signature in the fsi file. The compiler required me to define the Return method as member x.Return (_: unit) = empty
, but there doesn't seem to be a way to define the signature for that. member Return : unit -> AsyncSeq<'T>
gives the error Module 'FSharp.Control.AsyncSeq' requires a value 'member AsyncSeq.AsyncSeqBuilder.Return : unit -> AsyncSeq<'T>'
. This isn't an option unless we exclude this member from the signature file.
from fsharp.control.asyncseq.
I was really confused by this.
Found this thread and the other issue only after posting my own solution here
from fsharp.control.asyncseq.
This issue confused two of my teammates again last week (we're a team of only 6, so that really hurts). I've submitted a PR with my best effort to fix this issue.
from fsharp.control.asyncseq.
Related Issues (20)
- Best way to convert System.Collections.Generic.IAsyncEnumerable<'t> to AsyncSeq<'t>? HOT 6
- iterAsyncParallel and iterAsyncParallelThrottled may fail to cancel
- Add YieldFrom overload for seq<'a>? HOT 1
- Support .NET Standard 2.0 HOT 6
- Combination of groupBy and mapAsync involving access to the group causes deadlock. HOT 2
- [Feature] Can we have sort, sortBy, sortDescending and sortByDescending methods for AsyncSeq? HOT 3
- Update to use github actions
- netstandard2.1 only target forbids using the library in .net framework HOT 3
- asyncSeq not available HOT 6
- Currently broken with Fable HOT 7
- TaskSeq? HOT 6
- AsyncSeq deadlocks in Blazor WASM environment HOT 2
- AsyncSeq.mapAsyncParallel alternatives HOT 3
- Suggest to add a `chunkBy` function (to group and yield adjacent items sharing the same key) to the library HOT 4
- [General Question] HOT 2
- Deprecate `FSharp.Control.IAsyncEnumerable` in favor of the same type in BCL
- [Question] AsyncSeq.MapAsyncParallel HOT 2
- [Question] How to reduce Async<unit[]> HOT 1
- making AsyncSeq.mergeAll to accept array maybe
- Nuget package license link = 404 not found
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from fsharp.control.asyncseq.