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

Unexpected behavior with struct multicase unions #1678

Closed
cartermp opened this Issue Oct 28, 2016 · 21 comments

Comments

Projects
None yet
9 participants
@cartermp
Collaborator

cartermp commented Oct 28, 2016

The latest F# 4.1 compiler fails to compile either of the following code programs on .NET Core:

[<Struct>]
type Foo = Bar | Baz

[<EntryPoint>]
let main argv =
    printfn "Hello World from F#!"
    0 // return an integer exit code
[<Struct>]
type Foo = 
    | Bar of int
    | Baz of int

[<EntryPoint>]
let main argv =
    printfn "Hello World from F#!"
    0 // return an integer exit code

With this error:

FSC : error FS2014: A problem occurred writing the binary '/Users/phillip/scratch/test/obj/Debug/netcoreapp2.0/test.dll': Error in pass2 for type Program, error: Error in pass2 for type Foo, error: duplicate entry '.ctor' in method table [/Users/phillip/scratch/test/test.fsproj]

This is one rel/2.0.0 of the .NET CLI.

--- Old Issue ---

In VS "15" Preview 5, if you have the following:

[<Struct>]
type Shape =
    | Circle of radius: float
    | Square of side: double
    | Rectangle of sides: double*double

The following error occurs:

Error in pass2 for type Program, error: Error in pass2 for type Shape, error: duplicate entry '.ctor' in method table

If I remove the Square case , or change it to be of type int, it compiles.

@cartermp cartermp changed the title from Unexpected behavior with multicase union structs to Unexpected behavior with struct multicase unions Oct 31, 2016

@neildanson

This comment has been minimized.

Show comment
Hide comment
@neildanson

neildanson Nov 24, 2016

You also get the same in the case with something like this

[<Struct>]
type DU = A | B

I've not taken a look at the implementation of struct multi case unions, but I'm guessing that a unique constructor is built per signature of each case? That would explain why it fails for cases with duplicate signatures ?

neildanson commented Nov 24, 2016

You also get the same in the case with something like this

[<Struct>]
type DU = A | B

I've not taken a look at the implementation of struct multi case unions, but I'm guessing that a unique constructor is built per signature of each case? That would explain why it fails for cases with duplicate signatures ?

@cloudRoutine

This comment has been minimized.

Show comment
Hide comment
@cloudRoutine

cloudRoutine Nov 29, 2016

Contributor

I just ran into this issue too.

While it shows up in the build output, it does not in the error list

Contributor

cloudRoutine commented Nov 29, 2016

I just ran into this issue too.

While it shows up in the build output, it does not in the error list

@mistiara

This comment has been minimized.

Show comment
Hide comment
@mistiara

mistiara Jan 28, 2017

I just installed F# support for the latest VS2017 RC and hit this issue. It fails every time two or more cases have the same type.

Inspecting the IL emitted for struct unions I see a private constructor for each case. I think the easiest fix would be having a single constructor, with a parameter for every field.

mistiara commented Jan 28, 2017

I just installed F# support for the latest VS2017 RC and hit this issue. It fails every time two or more cases have the same type.

Inspecting the IL emitted for struct unions I see a private constructor for each case. I think the easiest fix would be having a single constructor, with a parameter for every field.

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Jan 28, 2017

Contributor

@mistiara Yes, we need to document this limitation, it is a bug in F# 4.1. We will fix it.

Contributor

dsyme commented Jan 28, 2017

@mistiara Yes, we need to document this limitation, it is a bug in F# 4.1. We will fix it.

@et1975

This comment has been minimized.

Show comment
Hide comment
@et1975

et1975 May 1, 2017

Is there any way to tell if it's in the fsharp/fsharp yet?

et1975 commented May 1, 2017

Is there any way to tell if it's in the fsharp/fsharp yet?

@vasily-kirichenko

This comment has been minimized.

Show comment
Hide comment
@vasily-kirichenko

vasily-kirichenko May 1, 2017

Contributor

@et1975 I believe fsharp/fsharp is dead. Build the compiler from this repo.

Contributor

vasily-kirichenko commented May 1, 2017

@et1975 I believe fsharp/fsharp is dead. Build the compiler from this repo.

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme May 2, 2017

Contributor

@et1975 I believe fsharp/fsharp is dead. Build the compiler from this repo.

No, not dead - just aligned. TBH it will never be "dead" because its existence forms part of the mission statement of the FSSF.

Right now, all the Mono distributions, FSharp.Compiler.Tools and FSharp.Compiler.Service packages are still packaged via integrations and tagging in that repo and/or fsharp/FSharp.Compiler.Service and other downstream packaging repos. The details are in https://github.com/fsharp/fsharp/blob/master/README.md

So fsharp/fsharp is really just a downstream packaging repo.

Unless absolutely critical (e.g. in preparing Mono releases) all contributions should go via this repo.

Contributor

dsyme commented May 2, 2017

@et1975 I believe fsharp/fsharp is dead. Build the compiler from this repo.

No, not dead - just aligned. TBH it will never be "dead" because its existence forms part of the mission statement of the FSSF.

Right now, all the Mono distributions, FSharp.Compiler.Tools and FSharp.Compiler.Service packages are still packaged via integrations and tagging in that repo and/or fsharp/FSharp.Compiler.Service and other downstream packaging repos. The details are in https://github.com/fsharp/fsharp/blob/master/README.md

So fsharp/fsharp is really just a downstream packaging repo.

Unless absolutely critical (e.g. in preparing Mono releases) all contributions should go via this repo.

@et1975

This comment has been minimized.

Show comment
Hide comment
@et1975

et1975 Jul 12, 2017

Is there any way to tell if this (or any other fix) has been released, in MS and/or Open distributions?

et1975 commented Jul 12, 2017

Is there any way to tell if this (or any other fix) has been released, in MS and/or Open distributions?

@neildanson

This comment has been minimized.

Show comment
Hide comment
@neildanson

neildanson Jul 12, 2017

@et1975 I'm not sure if there's an official way to tell, but I confirm this particular fix is in the latest f# compiler nuget package.

neildanson commented Jul 12, 2017

@et1975 I'm not sure if there's an official way to tell, but I confirm this particular fix is in the latest f# compiler nuget package.

@zpodlovics

This comment has been minimized.

Show comment
Hide comment
@zpodlovics

zpodlovics Jul 29, 2017

I just checked it with .NET Core 2.0 preview2 SDK (2.0.0-preview2-006497) and still hit the "duplicate entry '.ctor' in method table" issue with this example:

[<Struct>]
type DU = Case1 | Case2

zpodlovics commented Jul 29, 2017

I just checked it with .NET Core 2.0 preview2 SDK (2.0.0-preview2-006497) and still hit the "duplicate entry '.ctor' in method table" issue with this example:

[<Struct>]
type DU = Case1 | Case2
@cartermp

This comment has been minimized.

Show comment
Hide comment
@cartermp

cartermp Jul 29, 2017

Collaborator

I can confirm this with Preview 3 as well:

screen shot 2017-07-29 at 9 52 58 am

@KevinRansom does the compiler in the .NET Core SDK not contain the fix in #2811?

Collaborator

cartermp commented Jul 29, 2017

I can confirm this with Preview 3 as well:

screen shot 2017-07-29 at 9 52 58 am

@KevinRansom does the compiler in the .NET Core SDK not contain the fix in #2811?

@KevinRansom

This comment has been minimized.

Show comment
Hide comment
@KevinRansom

KevinRansom Jul 29, 2017

Contributor

@cartermp yes, that code is in both master and vs2017-rtm. It is in both the desktop and coreclr builds.

Contributor

KevinRansom commented Jul 29, 2017

@cartermp yes, that code is in both master and vs2017-rtm. It is in both the desktop and coreclr builds.

@KevinRansom

This comment has been minimized.

Show comment
Hide comment
@KevinRansom

KevinRansom Jul 29, 2017

Contributor

@et1975 Everything that was in master before 7/27/2017 is in the release branch for VS2017.3.

Merge PR: 4a312d3

The latest dotnet/cli compiler and nuget released Microsoft.FSharp.Compiler was built including those changes. The VS2017.3 rtm branch contain cherry-picks of the most important recent changes to master.

We don't have a great process for identifying when something goes from master to vs2017-rtm, that is usually controlled by the VS release cadence and the level of risk that can be allowed against the VS release. There is also the bar for the dotnet-cli to take into account, although we release to that out of VS2017-RTM via the nuget Microsoft.FSharp.Compiler.nuget package.

I'm sorry it's so difficult to figure out, releasing code to millions of users is a black-art and I am but a cog in the machine.

The rule of thumb I use is this ---
master branch --- will be released for the next + 1 VS and corresponding dotnet cli release.
vs2017-rtm --- will be released in the next VS and corresponding dotnet-cli release.

So far we have included code in VS2017, VS2017.2 and VS2017.3.

After a release has occurred, master will be merged into vs2017-rtm and relevant vs2017-rtm changes will be merged back into master.

All of the above is dependent on approvals from the VS release team and their goals for each release.

Contributor

KevinRansom commented Jul 29, 2017

@et1975 Everything that was in master before 7/27/2017 is in the release branch for VS2017.3.

Merge PR: 4a312d3

The latest dotnet/cli compiler and nuget released Microsoft.FSharp.Compiler was built including those changes. The VS2017.3 rtm branch contain cherry-picks of the most important recent changes to master.

We don't have a great process for identifying when something goes from master to vs2017-rtm, that is usually controlled by the VS release cadence and the level of risk that can be allowed against the VS release. There is also the bar for the dotnet-cli to take into account, although we release to that out of VS2017-RTM via the nuget Microsoft.FSharp.Compiler.nuget package.

I'm sorry it's so difficult to figure out, releasing code to millions of users is a black-art and I am but a cog in the machine.

The rule of thumb I use is this ---
master branch --- will be released for the next + 1 VS and corresponding dotnet cli release.
vs2017-rtm --- will be released in the next VS and corresponding dotnet-cli release.

So far we have included code in VS2017, VS2017.2 and VS2017.3.

After a release has occurred, master will be merged into vs2017-rtm and relevant vs2017-rtm changes will be merged back into master.

All of the above is dependent on approvals from the VS release team and their goals for each release.

@cartermp

This comment has been minimized.

Show comment
Hide comment
@cartermp

cartermp Jul 29, 2017

Collaborator

Hmmmm. Then it appears that @dsyme's PR didn't fix this duplicate ctor bug.

Collaborator

cartermp commented Jul 29, 2017

Hmmmm. Then it appears that @dsyme's PR didn't fix this duplicate ctor bug.

@KevinRansom

This comment has been minimized.

Show comment
Hide comment
@KevinRansom

KevinRansom Jul 29, 2017

Contributor

It's all fixed in VS2017.3
Desktop
image

Coreclr

C:\temp\acme>dotnet --info
.NET Command Line Tools (1.1.0)

Product Information:
 Version:            1.1.0
 Commit SHA-1 hash:  d6f4336106

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.14393
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\1.1.0

C:\temp\acme>type Program.fs
// Learn more about F# at http://fsharp.org

open System

[<Struct>]
type Shape =
    | Circle of radius: float
    | Square of side: double
    | Rectangle of sides: double*double

[<EntryPoint>]
let main argv =
    printfn "Hello World from F#!"
    0 // return an integer exit code

C:\temp\acme>dotnet restore
  Restore completed in 43.16 ms for C:\temp\acme\acme.fsproj.

C:\temp\acme>dotnet build
Microsoft (R) Build Engine version 15.3.409.57025 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  acme -> C:\temp\acme\bin\Debug\netcoreapp1.1\acme.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:01.05

C:\temp\acme>

Contributor

KevinRansom commented Jul 29, 2017

It's all fixed in VS2017.3
Desktop
image

Coreclr

C:\temp\acme>dotnet --info
.NET Command Line Tools (1.1.0)

Product Information:
 Version:            1.1.0
 Commit SHA-1 hash:  d6f4336106

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.14393
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\1.1.0

C:\temp\acme>type Program.fs
// Learn more about F# at http://fsharp.org

open System

[<Struct>]
type Shape =
    | Circle of radius: float
    | Square of side: double
    | Rectangle of sides: double*double

[<EntryPoint>]
let main argv =
    printfn "Hello World from F#!"
    0 // return an integer exit code

C:\temp\acme>dotnet restore
  Restore completed in 43.16 ms for C:\temp\acme\acme.fsproj.

C:\temp\acme>dotnet build
Microsoft (R) Build Engine version 15.3.409.57025 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  acme -> C:\temp\acme\bin\Debug\netcoreapp1.1\acme.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:01.05

C:\temp\acme>

@cartermp

This comment has been minimized.

Show comment
Hide comment
@cartermp

cartermp Jul 29, 2017

Collaborator

Does that work cross platform? I tested with the latest build of SDK 2.0 on macOS and repro'd.

Collaborator

cartermp commented Jul 29, 2017

Does that work cross platform? I tested with the latest build of SDK 2.0 on macOS and repro'd.

@KevinRansom

This comment has been minimized.

Show comment
Hide comment
@KevinRansom

KevinRansom Jul 30, 2017

Contributor

Should work fine cross platform.

Contributor

KevinRansom commented Jul 30, 2017

Should work fine cross platform.

@KevinRansom

This comment has been minimized.

Show comment
Hide comment
@KevinRansom

KevinRansom Jul 30, 2017

Contributor

Works on MAC-OS with the latest bits.

You need to use release/2.0.0 branch or rel/1.1.0 for attest bits. master is a bit whacky on dotnet/cli

MacBook-Pro:/Users/kevinr/temp/one> cat Program.fs
// Learn more about F# at http://fsharp.org

open System

[<Struct>]
type Shape =
    | Circle of radius: float
    | Square of side: double
    | Rectangle of sides: double*double

[<EntryPoint>]
let main argv =
    printfn "Hello World from F#!"
    0 // return an integer exit code
MacBook-Pro:/Users/kevinr/temp/one> dotnet build
Microsoft (R) Build Engine version 15.3.409.57025 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  one -> /Users/kevinr/temp/one/bin/Debug/netcoreapp2.0/one.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:01.35
MacBook-Pro:/Users/kevinr/temp/one> ````
Contributor

KevinRansom commented Jul 30, 2017

Works on MAC-OS with the latest bits.

You need to use release/2.0.0 branch or rel/1.1.0 for attest bits. master is a bit whacky on dotnet/cli

MacBook-Pro:/Users/kevinr/temp/one> cat Program.fs
// Learn more about F# at http://fsharp.org

open System

[<Struct>]
type Shape =
    | Circle of radius: float
    | Square of side: double
    | Rectangle of sides: double*double

[<EntryPoint>]
let main argv =
    printfn "Hello World from F#!"
    0 // return an integer exit code
MacBook-Pro:/Users/kevinr/temp/one> dotnet build
Microsoft (R) Build Engine version 15.3.409.57025 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  one -> /Users/kevinr/temp/one/bin/Debug/netcoreapp2.0/one.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:01.35
MacBook-Pro:/Users/kevinr/temp/one> ````
@zpodlovics

This comment has been minimized.

Show comment
Hide comment
@zpodlovics

zpodlovics Jul 30, 2017

@KevinRansom The example you posted (struct du with different unique type for each case) works fine with .NET Core preview2 (2.0.0-preview2-006497) running on Ubuntu 16.04. However it seems that the struct du the typeless/parameterless cases are not handled properly in the preview2 version of the sdk.

The compiler for this example at least provide an error why the struct du is not compilable (current struct du limitation):

open System

[<Struct>]
type DU =
    | Case1 of int
    | Case2 of int

[<EntryPoint>]
let main argv =
    printfn "Hello World from F#!"
    0 // return an integer exit code
dotnet run -c Release
/tmp/du/Program.fs(4,6): error FS3204: If a union type has more than one case and is a struct, then all fields within the union type must be given unique names. [/tmp/du/du.fsproj]

The build failed. Please fix the build errors and run again.

However for the following case, the compiler does not give any information why the compilation fails (current struct du limitations):

open System

[<Struct>]
type DU =
    | Case1
    | Case2

[<EntryPoint>]
let main argv =
    printfn "Hello World from F#!"
    0 // return an integer exit code
dotnet run -c Release
FSC : error FS2014: A problem occurred writing the binary '/tmp/du/obj/Release/netcoreapp2.0/du.dll': Error in pass2 for type Program, error: Error in pass2 for type DU, error: duplicate entry '.ctor' in method table [/tmp/du/du.fsproj]

The build failed. Please fix the build errors and run again.

zpodlovics commented Jul 30, 2017

@KevinRansom The example you posted (struct du with different unique type for each case) works fine with .NET Core preview2 (2.0.0-preview2-006497) running on Ubuntu 16.04. However it seems that the struct du the typeless/parameterless cases are not handled properly in the preview2 version of the sdk.

The compiler for this example at least provide an error why the struct du is not compilable (current struct du limitation):

open System

[<Struct>]
type DU =
    | Case1 of int
    | Case2 of int

[<EntryPoint>]
let main argv =
    printfn "Hello World from F#!"
    0 // return an integer exit code
dotnet run -c Release
/tmp/du/Program.fs(4,6): error FS3204: If a union type has more than one case and is a struct, then all fields within the union type must be given unique names. [/tmp/du/du.fsproj]

The build failed. Please fix the build errors and run again.

However for the following case, the compiler does not give any information why the compilation fails (current struct du limitations):

open System

[<Struct>]
type DU =
    | Case1
    | Case2

[<EntryPoint>]
let main argv =
    printfn "Hello World from F#!"
    0 // return an integer exit code
dotnet run -c Release
FSC : error FS2014: A problem occurred writing the binary '/tmp/du/obj/Release/netcoreapp2.0/du.dll': Error in pass2 for type Program, error: Error in pass2 for type DU, error: duplicate entry '.ctor' in method table [/tmp/du/du.fsproj]

The build failed. Please fix the build errors and run again.
@cartermp

This comment has been minimized.

Show comment
Hide comment
@cartermp

cartermp Jul 30, 2017

Collaborator

@KevinRansom

You need to use release/2.0.0 branch or rel/1.1.0 for attest bits

That's what I'm using. On rel/2.0.0:

[<Struct>]
type Foo = Bar | Baz

[<EntryPoint>]
let main argv =
    printfn "Hello World from F#!"
    0 // return an integer exit code

Fails:

FSC : error FS2014: A problem occurred writing the binary '/Users/phillip/scratch/test/obj/Debug/netcoreapp2.0/test.dll': Error in pass2 for type Program, error: Error in pass2 for type Foo, error: duplicate entry '.ctor' in method table [/Users/phillip/scratch/test/test.fsproj]
Collaborator

cartermp commented Jul 30, 2017

@KevinRansom

You need to use release/2.0.0 branch or rel/1.1.0 for attest bits

That's what I'm using. On rel/2.0.0:

[<Struct>]
type Foo = Bar | Baz

[<EntryPoint>]
let main argv =
    printfn "Hello World from F#!"
    0 // return an integer exit code

Fails:

FSC : error FS2014: A problem occurred writing the binary '/Users/phillip/scratch/test/obj/Debug/netcoreapp2.0/test.dll': Error in pass2 for type Program, error: Error in pass2 for type Foo, error: duplicate entry '.ctor' in method table [/Users/phillip/scratch/test/test.fsproj]

@cartermp cartermp reopened this Jul 30, 2017

@dsyme dsyme removed the Ready label Aug 5, 2017

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Aug 30, 2017

Contributor

@cartermp I tried

[<Struct>]
type Foo = Bar | Baz

on master and it works correctly for me

Contributor

dsyme commented Aug 30, 2017

@cartermp I tried

[<Struct>]
type Foo = Bar | Baz

on master and it works correctly for me

@dsyme dsyme closed this Aug 30, 2017

@cartermp cartermp modified the milestones: VS 2017 Updates, .NET Core 2.0, 15.5 Jun 9, 2018

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