Skip to content

Avoid re-typechecking 🚀╰(*°▽°*)╯#2389

Merged
facundominguez merged 15 commits into
ucsd-progsys:developfrom
gergoerdi:plguin/no-retypechecking
Oct 17, 2024
Merged

Avoid re-typechecking 🚀╰(*°▽°*)╯#2389
facundominguez merged 15 commits into
ucsd-progsys:developfrom
gergoerdi:plguin/no-retypechecking

Conversation

@gergoerdi

@gergoerdi gergoerdi commented Oct 16, 2024

Copy link
Copy Markdown
Contributor

This finally solves the long-standing problem of the LH plugin running the GHC typechecker. Note that this works without requiring anything new from GHC.

I guarantee you this will be the best PR you get from me this week :P

everything is done as late as possible, i.e. explicitly showing which steps
require re-typechecking and which steps require re-desugaring.
@gergoerdi gergoerdi force-pushed the plguin/no-retypechecking branch 2 times, most recently from 1053cf0 to 6c840ab Compare October 16, 2024 04:58
@gergoerdi gergoerdi changed the title Avoid re-typechecking Avoid re-typechecking 🚀╰(*°▽°*)╯ Oct 16, 2024
Comment thread liquidhaskell-boot/src/Language/Haskell/Liquid/GHC/Plugin.hs Outdated
@gergoerdi gergoerdi force-pushed the plguin/no-retypechecking branch from 6c840ab to 55455d4 Compare October 16, 2024 09:24

@facundominguez facundominguez left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gergoerdi! Quite an achievement. I've sprinkled some questions in parts that I don't fully understand.

Comment thread liquidhaskell-boot/src-ghc/Liquid/GHC/API/Extra.hs Outdated
Comment thread liquidhaskell-boot/src-ghc/Liquid/GHC/API/Extra.hs Outdated
Comment thread liquidhaskell-boot/src-ghc/Liquid/GHC/API/Extra.hs Outdated
Comment thread liquidhaskell-boot/src/Language/Haskell/Liquid/GHC/Plugin.hs
@gergoerdi gergoerdi force-pushed the plguin/no-retypechecking branch from 55455d4 to 8d45cc7 Compare October 16, 2024 11:02
@gergoerdi

gergoerdi commented Oct 16, 2024

Copy link
Copy Markdown
Contributor Author

Thanks @gergoerdi! Quite an achievement.

Yeah I'm quite proud of getting this to work.

@gergoerdi gergoerdi force-pushed the plguin/no-retypechecking branch from 8d45cc7 to 9b43230 Compare October 16, 2024 13:02
@gergoerdi gergoerdi force-pushed the plguin/no-retypechecking branch 2 times, most recently from 012b5b0 to 1d3d681 Compare October 17, 2024 07:31
@gergoerdi

Copy link
Copy Markdown
Contributor Author

Net -30 lines of code, that's how you know it's the good stuff!

Comment thread liquidhaskell-boot/src/Language/Haskell/Liquid/GHC/Plugin.hs
@gergoerdi gergoerdi force-pushed the plguin/no-retypechecking branch from 1d3d681 to 2c59a0a Compare October 17, 2024 10:53
@gergoerdi gergoerdi force-pushed the plguin/no-retypechecking branch from 2c59a0a to d67aef7 Compare October 17, 2024 11:02
= def &= help "Dump pre-normalized core (before a-normalization)"
&= name "dump-pre-normalized-core"
&= explicit
, insertCoreBreakPoints

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flag is still mentioned in the changelog for the next release.

@facundominguez

facundominguez commented Oct 17, 2024

Copy link
Copy Markdown
Collaborator

I contributed a commit to remove the boolean from the recursion in addNoInlinePragmasToBinds. I hope it doesn't change the meaning of your solution, while it encapsulates the concern of skipping binds to the function that deals with AbsBinds.

@gergoerdi

Copy link
Copy Markdown
Contributor Author

Don't know how to add comment to specific line on mobile...

I think it's nicer if you use a record pattern match to take the AbsBind apart, similar to how we take e.g. PatBinds apart.

Comment thread liquidhaskell-boot/src-ghc/Liquid/GHC/API/Extra.hs Outdated
@facundominguez facundominguez force-pushed the plguin/no-retypechecking branch from a22e6fb to e23da17 Compare October 17, 2024 13: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.

3 participants