Fable: Int64 regression: parsing negative numbers as positive ones

[Zaid-Ajaj]

Description

Detected this while running integration tests on remoting against Fable 2.0.2 and confirmed in the REPL
the function int64 parses negative numbers as without the negative sign

Repro code and current result

let a = int64 "5"
let b = int64 "-5"
printfn "%A" (a = 5L)   // true 
printfn "%A" (a = b)    // true 
printfn "%A" (b = -5L)  // false

Expected

let a = int64 "5"
let b = int64 "-5"
printfn "%A" (a = 5L)   // true
printfn "%A" (a = b)    // false
printfn "%A" (b = -5L)  // true

Please provide the expected and actual results.

Related information

• Fable version (dotnet fable --version): 2.0.2 and Current REPL (2.0.0-beta-005)
• Operating system: Win 10

[alfonsogarciacaro]

Thanks for checking this @Zaid-Ajaj! @ncave while debugging it seems the problem comes from this line which takes the third group of the regex match returned by isValid. However this group removes the negative sign. The line comes from this commit, is it intended? Can we fix it somehow? Thanks!

[Zaid-Ajaj]

@alfonsogarciacaro Confirmed that it is working now, thanks for the fix!

Quick question though, why was it sufficient to just update dotnet-fable to 2.0.3 and not Fable.Core too? Since there were changes made to the sources of Long.ts I expected that I would need to update Fable.Core but that wasn't needed and my library still depends on Fable.Core 2.0 does that meansif someone used dotnet-fable 2.0.2 to parse int64 with negative sign, their code would still fail or am I missing something obvious here?

[ncave]

@Zaid-Ajaj Confusing, right? Fable.Core is just the main ("core" if you will) bindings package, separate and different than fable-core (which is the javascript BCL implementation, bundled within dotnet-fable).
So yes, the issue was fixed in dotnet-fable 2.0.3, versions 2.0.2 and below would still have it.

[Zaid-Ajaj]

@ncave Thanks for the clarification! So if I understand correctly Fable.Core became just the binding and dotnet-fable now embeds the BCL implementation. I was still thinking in the Fable 0.7.x era (and I guess 1.3.x too) when Fable.Core was the BCL implementation and dotnet-fable applied the replacements, mappings etc.

However, this introduces the very small problem that libraries cannot pin a specifc dotnet-fable that they rely one. for example Fable.Remoting.Client depends on Fable.Core 2.0 but would fail parsing negative int64 if the user decides to use dotnet-fable v2.0.2 or an earlier version. am I correct?

[ncave]

@Zaid-Ajaj As far as I can remember (which is not too far), fable-core (the BCL) has always been separate and different from Fable.Core (the bindings), it's just that before it was distributed through npm, and now it's embedded in dotnet-fable.

So yes, because of that embedding, every time you compile your library with a different version of dotnet-fable, you will be bundling it with a (potentially) different version of the fable-core BCL. That's actually a feature, not a bug, in the sense that this way they're guaranteed to work together well (since dotnet-fable and fable-core are the same version and have been unit tested together).

[alfonsogarciacaro]

The name fable-core is used for historical reasons as in Fable 0.x it was a package distributed through npm that contained both JS files and Fable.Core.dll assembly. In Fable 1.x we moved to Nuget and the JS files were put in a fable-core folder within the Fable.Core package. However, most of the JS files are actually used by the Replacements module in the compiler. This meant that whenever there was a change in Replacements, we had to publish a new release for both Fable.Core and dotnet-fable (actually 3 releases because in Fable 1 the compiler was split in two packages but that's another story), even if Fable.Core hadn't changed at all. Then Paket had to make sure the versions for Fable.Core and dotnet-fable matched.

To simplify things, for Fable 2 I decided to put the fable-core JS files in the dotnet-fable package (maybe it would have been a good idea to change the fable-core name in that move) so we didn't have to make so many Fable.Core releases if the API hadn't changed. The main drawback as @Zaid-Ajaj points out is libraries cannot set a minimum compiler version by themselves. This was going to be an issue anyways when we likely move to the new dotnet CLI tools, as they won't share the project dependencies so it'll become more difficult to sync versions.

I don't know what's the best solution to this problem. Maybe we can add a function that can be called by libraries to set a minimum compiler version. What do you think? Would it be too cumbersome?

// In your library
Fable.Core.minimumCompilerVersion("2.0.3")

// Or maybe as an attribute
[<assembly: MinimumFableVersion("2.0.3")>]
do ()

[Zaid-Ajaj]

in the sense that this way they're guaranteed to work together well (since dotnet-fable and fable-core are the same version and have been unit tested together).

@ncave That part I understood but the problem was more that a library author cannot ensure that the library is compiled against matching versions of Fable.Core and dotnet-core. I can only pin the version of Fable.Core and hope that the user is using a matching version

@alfonsogarciacaro I think I get it, it has been a while since I've looked into the internals of Fable.

I don't think it is a big problem as the compiler isn't likely to update as often as libraries do, it is just a nice-to-jh As for the solution, I personally prefere not to pollute library code with compiler attributes, it is possible to add it in the .fsproj file a xml property?

<MinimumFableVersion>2.0.3</MinimumFableVersion>

Since Fable already has access to the sources of project files, it can read these and verify the versions

[ncave]

IMO dotnet-fable should know what is the minimum Fable.Core version it can compile against, and tell you if your project is referencing one that is too old, with a compile error or a warning. That would be even less friction or internal knowledge needed for library developers (unless I'm wrong on that).

[MangelMaxime]

I do agree with @ncave

It can be really hard for developpers to know which version of Fable.Core they should set as minimal. Personally, I don't really understand those version things and in general I use lastest whenever possible because I consider then the most stable.

[alfonsogarciacaro]

@ncave dotnet-fable already does this, but @Zaid-Ajaj's problem is the bug wasn't in Fable.Core but in the compiler itself, and he has no way to set a minimum compiler version from the library.

In Fable 1, Fable.Core and dotnet-fable packages were more or less in sync (mainly because the fable-core JS files were in the Fable.Core package) so setting a minimum Fable.Core version was a way to set a minimum version for the compiler. But this is not the case in Fable 2, and Fable.Core is expected to change much less often.

Just to be clear, my intention is to keep Fable.Core and dotnet-fable major and minor versions in sync. But this time it's trickier because the fix to make Fable.Remoting work is in a patch release (dotnet-fable 2.0.3). So it looks like we have two options:

• Add a mechanism so libraries can specify a minimum compiler version the user must run. For that we can use a property in the .fsproj as @Zaid-Ajaj suggests.
• Do nothing. If users encounter a problem, they should first try updating the compiler. This is not only for specific library problems but it may also affect their code, so it's always a sensible thing to do.

[MangelMaxime]

The second solution is more or less what we are already doing.

And people seems ok with that, every time someone as an issue my first is to make sure they use thé latest version