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

Getting record_insert: tried to extend but already exists when using patterns with duplicate bound variables #1906

Open
yannham opened this issue May 3, 2024 · 2 comments

Comments

@yannham
Copy link
Member

yannham commented May 3, 2024

Describe the bug

When using a pattern with duplicate bound variables, a match expression will fail at runtime with a strange and non-localized error: record_insert: tried to extend a record with the field y, but it already exists. This error leaks implementation details.

To Reproduce

nickel> match { { x = {y}, z = {y}} => 1 } { x.y = 1, z.y = 2}
error: record_insert: tried to extend a record with the field y, but it already exists

Expected behavior
One approach would be to forbid such patterns. For example, Rust forbid this and would error out with error[E0416]: identifier y is bound more than once in the same pattern. Same for OCaml: Error: Variable x is bound several times in this matching. This would technically be a breaking change, and this is why it was kept legal for destructuring (see #1324), but relying on this feature is very dubious.

Another solution would be to use update instead of insert when compiling patterns, so that we can handle several occurrences. But I don't have a good use-case for this, so I'm in favor of just making this illegal.

@aspiwack
Copy link
Member

aspiwack commented May 3, 2024

If you bind the same variable twice, you need some notion of equality. This is usually not very reasonable. Alternatively, you could merge the values that are bound to the different occurrences of y. This is a sane semantics, but probably a terribly confusing one.

It's probably quite a bit more reasonable to reject such patterns indeed.

@yannham
Copy link
Member Author

yannham commented May 3, 2024

If you bind the same variable twice, you need some notion of equality

The current implementation (in destructuring) is to do shadowing instead. Whether this is reasonable or not, that's another question...

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

No branches or pull requests

2 participants