Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Seq.init does not save values as suggested in description #14233

Open
abelbraaksma opened this issue Nov 3, 2022 · 5 comments
Open

Seq.init does not save values as suggested in description #14233

abelbraaksma opened this issue Nov 3, 2022 · 5 comments
Labels
Area-XmlDocs xmldoc generation, xmldoc content Bug good first issue help wanted Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Milestone

Comments

@abelbraaksma
Copy link
Contributor

The description and tooltip of Seq.init are as follows (emphasis mine):

Generates a new sequence which, when iterated, will return successive elements by calling the given function, up to the given count. Each element is saved after its initialization. The function is passed the index of the item being generated.

This suggests that for any sequence, the initializer is called only once and that successive access would use the saved value. I was inspecting the code and couldn't find how this was done and it turns out, it isn't. Not really, at least.

Repro steps

Basically, just this:

> let mutable i = 0;;
val mutable i: int = 0

> let s = Seq.init 10 (fun x -> i <- i + 1; i);;
val s: seq<int>

> Seq.last s;;
val it: int = 10

> Seq.last s;;
val it: int = 20

> i;;
val it: int = 20

Expected behavior

If the values from the generator were indeed saved, the initializer function would not be called again. However, the Seq.last returns different values, which means the mutable int I used gets updated again.

Actual behavior

The values aren't saved. At least not in the way I'd expect. They are saved momentarily for a call to Current, in such cases where you would use the IEnumerator<_>.Current directly. This saving itself is done in a Lazy<_> type to prevent problems if multiple threads read the value.

Known workarounds

Just ensure you don't write an expensive function as generator.

Related information

It's very well possible that this is "expected", and it is likely being used with side effects in practice. I'm bringing this up to discuss a better description for this function, as the way it is currently written, it confused me and possibly others.

@abelbraaksma
Copy link
Contributor Author

Btw, in contrast, note this for initInfinite, which juxtaposes the above. Which strengthens my feelings that, at least when originally written, the idea was to cache the generated values.

Generates a new sequence which, when iterated, will return successive elements by calling the given function. The results of calling the function will not be saved, that is the function will be reapplied as necessary to regenerate the elements. The function is passed the index of the item being generated.

The F# Core code for initInfinite does the exact same caching through Lazy as described above, there's no difference as far as I can tell.

@abonie
Copy link
Member

abonie commented Nov 4, 2022

Hmm... I agree the description is a bit misleading given the actual behavior, and I am not sure what actually was intended here. But I want to point out that there is Seq.cache that can be combined with Seq.init to get the behavior you expected.

@T-Gro
Copy link
Member

T-Gro commented Nov 4, 2022

=> I would suggest to keep the code (backwards compat), change the doc function and possibly mention within that doc comment to use Seq.cache to prevent recreating the members.

@T-Gro T-Gro added Area-Library Issues for FSharp.Core not covered elsewhere Area-XmlDocs xmldoc generation, xmldoc content good first issue help wanted and removed help wanted labels Nov 4, 2022
@vzarytovskii
Copy link
Member

=> I would suggest to keep the code (backwards compat), change the doc function and possibly mention within that doc comment to use Seq.cache to prevent recreating the members.

Yeah, we won't be able to change the behaviour.

@abelbraaksma
Copy link
Contributor Author

Exactly, that was why I brought it up:

I'm bringing this up to discuss a better description for this function, as the way it is currently written, it confused me and possibly others.

I’ll tinker over a proposal for a better description.

@0101 0101 added Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code. and removed Area-Library Issues for FSharp.Core not covered elsewhere Needs-Triage labels Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-XmlDocs xmldoc generation, xmldoc content Bug good first issue help wanted Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Projects
Status: New
Development

No branches or pull requests

5 participants