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

Move to ghc-lib-parser #645

Open
ndmitchell opened this issue May 26, 2019 · 13 comments

Comments

Projects
None yet
3 participants
@ndmitchell
Copy link
Owner

commented May 26, 2019

Given that haskell-src-exts is changing maintainer again https://www.reddit.com/r/haskell/comments/bswd7m/haskellcafe_haskellsrcexts_no_more_releases/ the plan is to get away from it and move to ghc-lib-parser. This ticket tracks the effort.

The rough game plan is:

  • Add ghc-lib-parser as a dependency
  • Parse all files with ghc-lib-parser
  • Make it a parse error if you can't parse with ghc-lib-parser
  • Rewrite the public API to make it not reference HSE values
  • Convert hints one by one to ghc-lib-parser
  • Stop parsing with HSE

The intent is to do this a combination of me and @shayne-fletcher-da.

@googleson78

This comment has been minimized.

Copy link
Contributor

commented Jun 4, 2019

Is there any way for random willing passer-bys to help with this effort?

@ndmitchell

This comment has been minimized.

Copy link
Owner Author

commented Jun 5, 2019

Thanks for the offer @googleson78. The patch to make it an error should be landing today. The change in the API is the big thing which is probably going to take a fair bit of effort and is hard to do, since it requires looking at and talking to all users. Once we get past that step the job of converting individual tests from HSE to GHC should be highly parallelisable, so any help at that stage would be most welcome! I'll comment on the ticket when we get there.

@ndmitchell

This comment has been minimized.

Copy link
Owner Author

commented Jun 28, 2019

@googleson78 we're now in the converting hints one by one. @shayne-fletcher-da is working on it, but it parallelises well, so if you want to take a crack at some hints, go for it. The first one has been done, #693.

@googleson78

This comment has been minimized.

Copy link
Contributor

commented Jun 28, 2019

I'll try doing NewType.hs.

@googleson78

This comment has been minimized.

Copy link
Contributor

commented Jun 29, 2019

I have two questions though:

  1. I see that stuff is getting renamed from X to XEx (Module -> ModuleEx). Should I also make a DeclEx which holds both a Decl_ and a HsDecl?
  2. What is the reason behind keeping both HIE and ghc-lib-parser types in the same place, instead of swapping out the HIE ones for ghc-lib-parser ones directly? I would be happy to read some discussions on the topic!
@ndmitchell

This comment has been minimized.

Copy link
Owner Author

commented Jun 29, 2019

  1. There's no guarantee that HSE and GHC consider programs to have the same number of Decl's so you can't do that. We probably need DeclOld and DeclNew as two separate hints, and slowly migrate from old to new. (Might make sense to leave the old one as is, introduce a new one, and flip names at the end)

  2. Not sure what you mean? Aim is to delete all HSE - current approach is to try and do it incrementally without a Big Bang.

@googleson78

This comment has been minimized.

Copy link
Contributor

commented Jun 29, 2019

WIP on NewType.hs:
#696

@googleson78

This comment has been minimized.

Copy link
Contributor

commented Jul 7, 2019

I'd also take a look at Naming.hs, but I don't know if I'll have too much time before the coming weekend, if someone else wants to do it before then.

@googleson78

This comment has been minimized.

Copy link
Contributor

commented Jul 7, 2019

Question:

Why does NewType.hs give a hint for a single declaration, whereas Naming.hs gives hints for an entire module (as far as I can tell)?

@googleson78

This comment has been minimized.

Copy link
Contributor

commented Jul 7, 2019

Question2:

Do you want to have Named instances for ghc-lib-parser stuff, like it is currently with HSE?

@ndmitchell

This comment has been minimized.

Copy link
Owner Author

commented Jul 8, 2019

Naming tries to avoid suggesting names that are already used so it has to work at the whole module level.

Where you use the Named type class or not is your choice. It turned out to be useful for HSE, but if GHC works better another way I'm happy. Although I would default to doing the same again since that's probably easiest since we know it works.

@googleson78

This comment has been minimized.

Copy link
Contributor

commented Jul 13, 2019

Here's a WIP on Naming.hs - #708

@shayne-fletcher-da

This comment has been minimized.

Copy link
Contributor

commented Jul 17, 2019

@googleson78 @ndmitchell comments done, heads-up : taking on imports next.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.