dotnet / fsharp

The F# compiler, F# core library, F# language service, and F# tooling integration for Visual Studio
https://dotnet.microsoft.com/languages/fsharp
MIT License
3.83k stars 772 forks source link

Units of measure can affect compiled form of integral ranges #17046

Open brianrourkeboll opened 3 months ago

brianrourkeboll commented 3 months ago

When looking into #17025, I found that the presence of units of measure can affect the compiled form of the range (..) and range-step (.. ..) operators. This applies to their use in bare for-loops as well as in computed collections.

When start and finish (for (..)) and start, step, and finish (for (.. ..)) are all literal values, local variables, or parameter values, the compiled form with units of measure is the same as it is without units of measure.

But when any of start, step, or finish is a function call, method call, property access, etc., the compiled form with units of measure diverges from the compiled form without units of measure.

That is, these pairs are compiled identically (as expected):

for n in 1..1..10 do ignore (float n ** float n)
for n in 1<m>..1<m>..10<m> do ignore (float n ** float n)
[1..1..10]
[1<m>..1<m>..10<m>]
[|1..1..10|]
[|1<m>..1<m>..10<m>|]

But these pairs are compiled differently:

let f1 g = for n in g ()..1..10 do ignore (float n ** float n)
let f2 g = for n in g ()..1<m>..10<m> do ignore (float n ** float n)
let f3 g = [g ()..1..10]
let f4 g = [g ()..1<m>..10<m>]
let f5 g = [|g ()..1..10|]
let f6 g = [|g ()..1<m>..10<m>|]

This divergence is happening in the compiler somewhere before for-loops or computed collections are optimized, which means that neither the longstanding optimization for integer for-loops over ranges with a step of 1 or -1 nor the new optimizations introduced in #16650 are applied in certain scenarios when units of measure are involved.

I don't believe that this should be the case. Ideally, the presence of units of measure should not affect the compiled form of these constructs.

T-Gro commented 2 months ago

This was fixed by #17048 right?

brianrourkeboll commented 2 months ago

This was fixed by #17048 right?

No. This is actually a longstanding difference that also affects the older optimizations (see the second SharpLab link from above).