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

Disambiguate optional labels #6798

Merged
merged 2 commits into from
Jun 7, 2024
Merged

Conversation

cristianoc
Copy link
Collaborator

When disambiguating record types, there's a check that all the labels are supplied when constructing a record. While not supplying all the labels is supported in case of optional labels, the order of disambiguation is affected by the presence of optional labels.

Example:

type t1 = {x:int, y:int}
type t2 = {x:int, y:int, z?:int}

let v = {x:3, y:4}

Currently v has type t1, while it's perfectly fine for it to have type t2. In particular, the normal shadowing behaviour that applies without optional labels, does not happen. (If you remove z from the second type definition, then the normal shadowing happens, and v gets type t2.

This wip changes the disambiguation so that supplying at least all the mandatory labels is enough in disambiguation.

The change also addresses the issue #6752 of spurious warning of unused open.

@cristianoc
Copy link
Collaborator Author

CC @zth @cknitt see difference in disambiguation of record value.
Implementation still to be cleaned up.

When disambiguating record types, there's a check that all the labels are supplied when constructing a record.
While not supplying all the labels is supported in case of optional labels, the order of disambiguation is affected by the presence of optional labels.

Example:

```res
type t1 = {x:int, y:int}
type t2 = {x:int, y:int, z?:int}

let v = {x:3, y:4}
```

Currently `v` has type `t1`, while it's perfectly fine for it to have type `t2`.
In particular, the normal shadowing behaviour that applies without optional labels, does not happen. (If you remove `z` from the second type definition, then the normal shadowing happens, and `v` gets type `t2`.

This wip changes the disambiguation so that supplying at least all the mandatory labels is enough in disambiguation.

The change also addresses the issue #6752 of spurious warning of unused open.
@cristianoc cristianoc force-pushed the disambiguate_optional_labels branch from b632d73 to dbf88c2 Compare June 6, 2024 09:44
@cristianoc cristianoc changed the title WIP: disambiguate optional labels Disambiguate optional labels Jun 6, 2024
@cristianoc cristianoc requested a review from zth June 6, 2024 09:45
@cristianoc cristianoc enabled auto-merge (rebase) June 7, 2024 06:59
@cristianoc cristianoc merged commit 2c67459 into master Jun 7, 2024
18 checks passed
@cknitt cknitt deleted the disambiguate_optional_labels branch June 16, 2024 04:47
@fhammerschmidt fhammerschmidt modified the milestone: v11.1.3 Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants