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

Fix: destructing record field in pattern #1436

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ulugbekna
Copy link
Contributor

First commit adds a test showing the problem; the second one fixes it.

I hope I'm not leaving out some edge cases, went with the seemingly easiest fix.

@trefis
Copy link
Contributor

trefis commented Feb 8, 2022

Doesn't your code break a call to destruct on x on the following example:

type r = { x : int }
type r_out = { r : r; y : string }

let f ({ r = x ; y } : r_out) = ()

by producing (in the end) { r = r = { x }; y }?

Generally to detect punning you'll want to have a look at the location of the nodes.

Also, I'm surprised that the behavior depends on the content of the field. 🤷

@ulugbekna
Copy link
Contributor Author

ulugbekna commented Feb 8, 2022

Indeed, the fix is incorrect. I’ll see what to do about it

Also, I'm surprised that the behavior depends on the content of the field. 🤷

I don’t quite understand what you mean

thanks for quick review :-)

@voodoos voodoos marked this pull request as draft November 23, 2022 16:46
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.

None yet

2 participants