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

Enable OverloadedRecordDot, NoFieldSelectors and DuplicateRecordFields #172

Merged
merged 2 commits into from Jan 17, 2023

Conversation

arcz
Copy link
Collaborator

@arcz arcz commented Jan 15, 2023

Description

OverloadedRecordDot is available from 9.2 and allows for nice records access without lens. I think the next step would be to move to something more modern for lens such as generic-lens or optics-core. We can then drop the underscores from records and access them through the dot syntax and reach for lens for more complex cases.

Checklist

  • tested locally
  • added automated tests
  • updated the docs
  • updated the changelog

src/EVM.hs Show resolved Hide resolved
@d-xo
Copy link
Collaborator

d-xo commented Jan 17, 2023

This is great, awesome to see such advanced technology arrive in haskell after 30 years 😅

Supportive of the longer term refactoring plans to move away from lens. I haven't personally used optics, but I would definitely be interested in trying it out as I have always found the type signatures and documentation in the og lens library extremely hard to parse. We can also probably tidy the field names in quite a few records since duplicates are allowed now.

One concern I do have is how this will impact any downstream consumers who do not have OverloadedRecordDot and friends enabled? I guess at the very least this means we can't compile with ghc < 9.2 (not a blocker, just curious).

@arcz
Copy link
Collaborator Author

arcz commented Jan 17, 2023

If needed, for GHC < 9.2, we could add this for OverloadedRecordDot https://hackage.haskell.org/package/record-dot-preprocessor, the NoFieldSelectors extension can be simply dropped and DuplicateRecordFields is available in the older compilers.

@d-xo
Copy link
Collaborator

d-xo commented Jan 17, 2023

I think having a min dep on 9.2 is fine for now, but good to know that we have options if it becomes problematic. Thanks for the pr!

@d-xo d-xo merged commit 780e74a into ethereum:main Jan 17, 2023
@arcz arcz deleted the recorddot branch January 17, 2023 15:23
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