Skip to content
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

Reword or better explain unique names on Struct DUs #5236

Open
Tracked by #1103
isaacabraham opened this issue Jun 26, 2018 · 3 comments
Open
Tracked by #1103

Reword or better explain unique names on Struct DUs #5236

isaacabraham opened this issue Jun 26, 2018 · 3 comments
Labels
Area-Diagnostics mistakes and possible improvements to diagnostics Feature Improvement
Milestone

Comments

@isaacabraham
Copy link
Contributor

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
Copy link

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
Copy link
Contributor

@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
Copy link
Contributor

Related or duplicate: #3648.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Diagnostics mistakes and possible improvements to diagnostics Feature Improvement
Projects
Status: New
Development

No branches or pull requests

6 participants