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

Any bug in QueueList? #4722

Closed
zhuangzhuang opened this issue Apr 13, 2018 · 8 comments · Fixed by #4727
Closed

Any bug in QueueList? #4722

zhuangzhuang opened this issue Apr 13, 2018 · 8 comments · Fixed by #4727
Labels
Bug Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Milestone

Comments

@zhuangzhuang
Copy link

zhuangzhuang commented Apr 13, 2018

https://github.com/Microsoft/visualfsharp/blob/5daaf8294ab0adcf1a19a77935966e5b3a30c32e/src/fsharp/QueueList.fs#L37

member x.Append(ys:seq<_>) = QueueList(firstElements, (List.rev (Seq.toList ys) @ lastElementsRev), numLastElements+Seq.length ys))

counterexamples:

let headQueue = ofSeq [1..5]
let tailQueue = ofSeq [1..100000]
let temp = append headQueue tailQueue
temp.ToList() |>ignore // call List.rev inside
temp.ToList() |> ignore // call List.rev again
@zhuangzhuang
Copy link
Author

miss understand, this is not a bug.
please close this issues.
sorry

@abelbraaksma
Copy link
Contributor

abelbraaksma commented Apr 13, 2018

@zhuangzhuang you can close your own bug by clicking the close button ;)

@zaaack
Copy link

zaaack commented Apr 16, 2018

@zhuangzhuang As we discussed yesterday, this is actually a bug here, could you please reopen it?

@zhuangzhuang zhuangzhuang reopened this Apr 16, 2018
forki added a commit to forki/visualfsharp that referenced this issue Apr 16, 2018
@forki
Copy link
Contributor

forki commented Apr 16, 2018

@zaaack & @zhuangzhuang how did you spot this? Is there observable behaviour?

Anyway: fix is in #4727

@zaaack
Copy link

zaaack commented Apr 16, 2018

@forki Not me, it's @zhuangzhuang find this, I guess he found it by accident when reading vf's source code?

@forki
Copy link
Contributor

forki commented Apr 16, 2018

mhm looks like I fixed another issue. It still does 2 reverse operations.

@forki
Copy link
Contributor

forki commented Apr 19, 2018

@KevinRansom please reopen. The double reverse issue is not fixed.

@KevinRansom KevinRansom reopened this Apr 19, 2018
@cartermp cartermp added Area-Compiler Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code. labels Apr 20, 2018
@cartermp cartermp added this to the Unknown milestone Aug 25, 2018
@cartermp cartermp modified the milestones: Unknown / not bug, Backlog May 23, 2019
@dsyme dsyme added the Bug label Sep 1, 2020
@dsyme
Copy link
Contributor

dsyme commented Mar 31, 2022

@forki Closing as this is internal to the compiler and would need an observable external behaviour in order to fix

@dsyme dsyme closed this as completed Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants