Skip to content

Rename plugin doesn't work with NamedFieldPuns #2970

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
santiweight opened this issue Jun 18, 2022 · 4 comments · Fixed by #3013
Closed

Rename plugin doesn't work with NamedFieldPuns #2970

santiweight opened this issue Jun 18, 2022 · 4 comments · Fixed by #3013
Labels
component: hls-rename-plugin type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..

Comments

@santiweight
Copy link
Collaborator

santiweight commented Jun 18, 2022

The environment doesn't matter here: just a plugin bug...

Steps to reproduce

Rename the following field to something else using the rename plugin

{-# LANGUAGE NamedFieldPuns #-}
newtype Foo = Foo { field :: Int }

unFoo :: Foo -> Int
unFoo Foo{field} = field

Expected behaviour

I think the best UX should be that renames compose with bindings incurred by NamedFieldPuns

Actual behaviour

The output is instead:

{-# LANGUAGE NamedFieldPuns #-}
newtype Foo = Foo { bleh :: Int }

unFoo :: Foo -> Int
unFoo Foo{bleh} = field

i.e. the field reference in the binding site is renamed, but not the subsequent reference on the RHS of unFoo

@santiweight santiweight added status: needs triage type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc.. labels Jun 18, 2022
@santiweight
Copy link
Collaborator Author

I'm happy to take this on, but it's not immediately clear to me what the solution is. The result returned from hie-db is a RefRow, which appears only to be a location and the OccName.

I see two options:

  1. Have NamedFieldPuns return the RHS reference to field as a reference to field. I don't think this is a good solution, because I don't think the RHS reference is a reference to the field; it's more a reference to the punned field binding.
  2. Do a subsequent pass for each punned field binding. In other words, if RefRow returned information about whether this reference is a punned field, then we could do a second pass on these punned fields.

I'm happy to take this on: someone more hie-savvy please tell me the preferable option (or some other option I didn't think/know of)

@July541
Copy link
Collaborator

July541 commented Jun 19, 2022

Same as #2915?

@michaelpj
Copy link
Collaborator

cc @OliverMadine

@OliverMadine
Copy link
Collaborator

Sorry for the delay in following up on this.
This is a different issue to  #2915. I will open a PR shortly.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
component: hls-rename-plugin type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants