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.9k stars 783 forks source link

Reword or better explain unique names on Struct DUs #5236

Open isaacabraham opened 6 years ago

isaacabraham commented 6 years ago

What

When creating a multi-case struct DU, if there is more than one field across all cases, each fields across all cases must have explicits, unique name e.g.

[<Struct>]
type Foo =
    | Hi of bool
    | Lo of int

leads to

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

Why

The error is misleading IMHO. Firstly, it's not clear to me under what circumstances this error appears e.g. this compiles fine:

[<Struct>]
type Foo =
| Hi of bool
| Lo of int * bool

Nonetheless, the main issue is that the error message makes me think that all named fields must be unique i.e. surely the compiler can generate unique names for the other ones?

Yet this works:

[<Struct>]
type Foo =
| Hi of bool
| Lo of bar:int

But this doesn't


[<Struct>]
type Foo =
| Hi of bool
| Lo of bar:int
| Med of int

In short, the compiler error message doesn't clearly explain when this error occurs, or how to resolve it.

How

The error message should clearly state how this error occurred (I would suggest something but I'm not entirely clear myself at the moment), and it should ideally give a hint as to how to fix the issue with the offending fields that have duplicated names.

KathleenDollard commented 2 years ago

Can we consider whether is a problem with messaging or a problem with behavior or a problem with behavior that we will solve for now with messaging?

smoothdeveloper commented 2 years ago

@KathleenDollard, AFAIU/R, the ideal struct DU implementation would try to minimize the number of fields in a single opaque struct type, but currently the compiler puts a separate field, even if they could be technically coallesced.

If we fixed this, then the message would probably disappear, and we would probably need to handle other error cases.

I've not seen Struct DU frequently used and I think they came in in their current shape to match updating the compiler to support struct records.

@dsyme, do you think there will be changes to the underlying representation of struct DU along what I refer to in first paragraph?

Basically, can it be optimized for


[<Struct>]
type Foo =
    | Hi of a:bool
    | Lo of b:bool

to have just the tag and the field in the IL?

if not, is there anything to do about this error message? it just explains that the compiler needs field names for all the DU members, I'm not sure what to change about the message, which gets fixed after naming the fields (so the message is fine to me).

abelbraaksma commented 2 years ago

Related or duplicate: #3648.