"Rational Units of Measure" changes encoding of existing F# 3.x metadata

[dsyme]

The "Rational Units of Measure" feature changes encoding of existing F# 3.x metadata.

This causes strong failure when using FSharp.Core 4.4.0.0 with F# 3.x tools (was: using fsharp.sln with VS2013 Update 3/4)

For example, when using src\fsharp.sln with Visual Studio 2013 I get no Intellisense in any project. This is relatively new - previously I've always been able to use this combination to develop F#.

[forki]

It's not just you. I have the same issue lately and this stopped me from working on #50

[dsyme]

OK, I've found the cause of this (it is not the above callstack)

Error reading/writing metadata for the F# compiled DLL 'C:\\GitHub\\dsyme\\visualfsharp\\Debug\\net40\\bin\\FSharp.Core.dll'. Was the DLL compiled with an earlier version of the F# compiler? (error: 'u_measure_expr').

The problem is that referencing the new FSharp.Core 4.4.0.0 causes a silent hard failure when using F# 3.x tools. This is related to the metadata format extensions for the "rational units of measure" feature.

As discussed in the CodePlex thread on "nativeptr intrinsics", we would very much like if referencing FSharp.Core 4.4.0.0 doesn't cause a hard failure for previous versions of the F# tools. Our understanding was that the new metadata extensions for rrational units of measure were not used in FSharp.Core 4.4.0.0 and would only be used when a rational unit of measure was actually used somewhere in the code. However it seems this is not the case - in particular it is likely that the metadata for "sqrt" uses this new format (perhaps unnecessarily).

I'll discuss this with Andrew Kennedy to see if we can pinpoint what the issue is.

[dsyme]

(I edited my initial description removing the original callstack which was unrelated)

[dsyme]

Note that we need to make sure that the pickled metadata for FSharp.Core doesn’t use the new case #5 of the pickled metadata format

Indeed we need to make sure that this case is not emitted for any existing F# 3.x code, and only for F# 4.0 code that uses the Rational Units of Measure feature.

Note that the F# 4.x toolset must be able to compile and emit F# 3.x-toolset-compatible code that can be consumed by existing F# 3.x toolsets, e.g. when compiling a DLL that references FSharp.Core 4.3.1.0 and that DLL is used in a nuget package referenced by F# 3.x tools. It is likely we need to add more regression testing to ensure that the F# 4.x toolset is fully usable in this way,

[dsyme]

Andrew Kennedy is looking at this. When I investigated a bit more, the line

| MeasureRationalPower(x,q) -> p_byte 5 st; p_measure_expr x st; p_rational q st

is being called with

q = 2/1
q = -1/1

So it looks like we just need to encode all cases where "q = an integer" using the existing metadata F# 3.x constructs.

[dsyme]

(I hacked in a fix to this into my local copy coping with q = 2/1 and q = -1/1 and that fixed the original problem "using fsharp.sln with Visual Studio 2013", so I think we've found the problem :) )

[forki]

could you please create a PR for this (maybe flagged as WIP), I'd like to test it.

[dsyme]

Andrew is working on it. Before this line:

| MeasureRationalPower(x,q) -> p_byte 5 st; p_measure_expr x st; p_rational q st

I added these three lines, then recompiled "proto", "library" and "compiler"

| MeasureRationalPower(x,q) when Rational.GetNumerator q = 1 && Rational.GetDenominator q = 1 -> p_measure_expr x st
| MeasureRationalPower(x,q) when Rational.GetNumerator q = 2 && Rational.GetDenominator q = 1 -> p_measure_expr (MeasureProd(x,x)) st
| MeasureRationalPower(x,q) when Rational.GetNumerator q = -1 && Rational.GetDenominator q = 1 -> p_measure_expr (MeasureInv(x)) st
| MeasureRationalPower(x,q) -> p_byte 5 st; p_measure_expr x st; p_rational q st

Only the second, third and fourth lines are needed

[latkin]

It is likely we need to add more regression testing to ensure that the F# 4.x toolset is fully usable in this way

Yes, multitargeting and cross-targeting (to 3.1 or 3.0, or net20 or portable) is a weak spot in the test coverage right now. Doubly bad because downlevel targeting to 3.1 or 3.0 is a critical scenario for a lot of people. There are ways to achieve this with the current test framework (it's done internally for net20 at least), it just needs some attention.