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

Mutable incorrectly optimised outside of loop #6895

Closed
rca22 opened this issue May 29, 2019 · 5 comments
Closed

Mutable incorrectly optimised outside of loop #6895

rca22 opened this issue May 29, 2019 · 5 comments

Comments

@rca22
Copy link

rca22 commented May 29, 2019

In F# 4.6.2 it appears that a mutable defined inside a for loop has it's definition incorrectly optimised so that it is effectively defined outside of the iteration.
This can be demonstrated with the following code.

let bug() = 
  for a in [1;2;3;4] do
    let mutable x = null
    if x = null then
      x <- HashSet<int>()
    x.Add a |> ignore
    let expected = [a]
    let actual = List.ofSeq x
    if expected <> actual then
      failwith "Bug"

The expected behaviour is clear from the function above. What actually happens is that the above code is compiled as if the line 'let mutable x = null' comes before the line 'for a in [1;2;3;4] do' and after the seconditeration, x holds two integers, not one, and fails.

This bug appears to have been introduced in VS2019/F# 4.6.2 - the same code in VS2017/F# 4.5.4 works fine.

My workaround for this is to replace the mutable with a reference cell. The code below does not throw.

let not_a_bug() = 
  for a in [1;2;3;4] do
    let x = ref null
    if (!x) = null then
      x := HashSet<int>()
    (!x).Add a |> ignore
    let expected = [a]
    let actual = List.ofSeq (!x)
    if expected <> actual then
      failwith "Bug"
  • Operating system : Windows 10
  • .NET Runtime kind (.NET Core, .NET Framework, Mono): .Net Framework 4.8
  • Editing Tools: VS 2019.
@cartermp
Copy link
Contributor

cc @TIHan could this be due to the for in do optimizations for Span?

@TIHan
Copy link
Contributor

TIHan commented May 29, 2019

It might, but this code is not trying to iterate over a Span, or even an array. In fact, no for in do optimizations will be used.

This is a pretty bad one. I can look at it.

@cartermp
Copy link
Contributor

@TIHan You're correct, this has nothing to do with that optimization.

This is a regression in F# 4.6.

You can see this not reproduce in F# 4.0 here: https://repl.it/@cartermp/NeighboringClearcutHertz

I also confirmed that does not reproduce if I pin my .NET Core SDK to 2.1.300 (released 2018-05-30), which is F# 4.5.

Pinning to 2.2.203 (released 2019-04-09), which is the first .NET Core that had F# 4.6 in it, I can reproduce this.

@cartermp
Copy link
Contributor

Adding @dsyme as this may be related to the various optimization work done for F# 4.6 (that @TIHan hadn't touched)

@cartermp
Copy link
Contributor

Closed as resolved in 16.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants