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

Plugin generates unhygienic code #162

Closed
parsonsmatt opened this issue Oct 8, 2024 · 2 comments
Closed

Plugin generates unhygienic code #162

parsonsmatt opened this issue Oct 8, 2024 · 2 comments

Comments

@parsonsmatt
Copy link
Contributor

parsonsmatt commented Oct 8, 2024

Our custom prelude exports error as an alias for errorRequireCallStack, which causes the large-record plugin to fail with this compiler error message:

src/Handler/Perks/Types.hs:16:14-25: error: [GHC-64725]
    • Add RequireCallStack to your function context or use provideCallStack
    • In the expression:
        error
          "Pattern match failure in vectorToPerksSecrets: vector with invalid number o
f elements."
      In a case alternative:
          _ -> error
                 "Pattern match failure in vectorToPerksSecrets: vector with invalid n
umber of elements."
      In the expression:
        case Data.Record.Plugin.Runtime.anyArrayToList _x of

We can work-around this by hiding custom error and using Prelude.error but I think it'd be better if the plugin were hygienic about the terms it uses

edsko added a commit that referenced this issue Oct 15, 2024
@edsko
Copy link
Collaborator

edsko commented Oct 15, 2024

Using Exact names in source plugins in a pain in the ass: you need to provide the location of where the symbol is defined; you cannot provide the location of a module that merely re-exports it. For this reason large-records comes with a Data.Record.Plugin.Runtime module that defines type aliases and term aliases for nearly everything we need. Unfortunately, we cannot define class aliases; if you do

type MyEq = Eq

then ghc will not accept

instance MyEq T where ..

This means that for classes we must define the precise location of the class; for the Prelude names I wasn't doing this, and was just using whatever was in scope, to sidestep this issue. I have now pushed a fix to #164 that imports these names from GHC.Classes (Eq and Ord) and GHC.Show (Show) instead. Unfortunately, the former comes from ghc-prim, so the (small) price to pay is that that is now yet another dependency to add (for a total of three: ghc-prim, large-records, and record-hasfield). Once that PR passes CI I will make a release.

edsko added a commit that referenced this issue Oct 15, 2024
@edsko edsko closed this as completed in 6ed7129 Oct 15, 2024
@edsko
Copy link
Collaborator

edsko commented Oct 15, 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

No branches or pull requests

2 participants