Use ghc instead of ghc-lib-parser when possible

[sol]

This significantly reduces the build time with recent version of GHC.

[amesgen]

That definitely makes sense in general, and other tools like hlint also offer a flag to do this, though hlint disabled using ghc by default when possible, but I do not yet fully understand why they chose to do so: ndmitchell/hlint#1410

[mrkkrp]

I applied the gitignore-related change separately in #953.

We are a bit undecided about the main part though. One thing we do not want to do for sure is introducing a cabal flag because then we would need a bigger CI matrix and longer CI times. If we force ghc for 9.4 then there are two reservations:

• Ormolu might stop being agnostic to whether ghc-lib-parser or ghc in used at some point, e.g. if a change like Allow GHC DynFlags to passed directly to the API #639 is implemented.
• Sometimes there are GHC parsing bugs on just one patch version. E.g. suppose there is a bug in 9.4.1 which is fixed in 9.4.2, and you are for some reason forced to compile Ormolu with 9.4.1, then after this PR you can't get a working Ormolu anymore.

[sol]

I applied the gitignore-related change separately in #953.

Thanks for that! That makes sense!

We are a bit undecided about the main part though. One thing we do not want to do for sure is introducing a cabal flag

👍 less flags are always a good thing, especially if they are manual!

• Ormolu might stop being agnostic to whether ghc-lib-parser or ghc in used at some point, e.g. if a change like Allow GHC DynFlags to passed directly to the API #639 is implemented.

I would hope this can be addressed by CI. Testing e.g. 9.2.5 and 9.4.4 on CI should make sure that both ghc-lib-parser and ghc work.

• Sometimes there are GHC parsing bugs on just one patch version. E.g. suppose there is a bug in 9.4.1 which is fixed in 9.4.2, and you are for some reason forced to compile Ormolu with 9.4.1, then after this PR you can't get a working Ormolu anymore.

From what I understand you have the same issue if you accept multiple versions of ghc-lib-parser, right now we are accepting 9.4.*. From what I understand making sure that we have "parity" between the versions of ghc and ghc-lib-parser that we allow can address this.

Examples you ask?

Right now we have:

if impl(ghc >=9.4 && <9.5)
  build-depends: ghc
else
  build-depends: ghc-lib-parser >=9.4 && <9.5

If we discovered a bug in 9.4.1 then we would change this to:

if impl(ghc >=9.4.2 && <9.5)
  build-depends: ghc
else
  build-depends: ghc-lib-parser >=9.4.2 && <9.5

If at some later point we decided that everybody should be on the same single lastest version then we would change this again to:

if impl(ghc ==9.4.4)
  build-depends: ghc
else
  build-depends: ghc-lib-parser ==9.4.4.*

Does this make sense?

All that said, I am not committed to this change by any means. It just seemed like an easy win in terms of build times. But I'm OK either way.

[mrkkrp]

I would hope this can be addressed by CI. Testing e.g. 9.2.5 and 9.4.4 on CI should make sure that both ghc-lib-parser and ghc work.

That is true, but CPP might be necessary in that case.

From what I understand you have the same issue if you accept multiple versions of ghc-lib-parser, right now we are accepting 9.4.*. From what I understand making sure that we have "parity" between the versions of ghc and ghc-lib-parser that we allow can address this.

I think it is easier to just specify a different version of ghc-lib-parser (the one where the problem is fixed, this is even possible without editing the .cabal file) as opposed to switching GHC version that the user happens to have. It is true that with a correction in .cabal file like you show above the problem can be mitigated, so it is not a super strong point.

In general, I guess we just prefer simplicity and are not very much effected by longer build times.

[sol]

CPP might be necessary in that case

How is that?

I was under the impression that the packages are interchangeable as long as you do not exercise too much of ghc (say ghc-lib-parser strictly being a subset of ghc).

But I might be wrong here.

I think it is easier to just specify a different version of ghc-lib-parser (the one where the problem is fixed, this is even possible without editing the .cabal file)

Do you mean that a user would do this themselves by manually constraining the version?

(say by passing --constraint="ghc-lib-parser ==9.4.4.* to cabal or alternatively constraining the version through other means like stack or nix)

In general, I guess we just prefer simplicity and are not very much effected by longer build times.

That's fine with me. Please feel free to close this PR.

[mrkkrp]

How is that?

I was under the impression that the packages are interchangeable as long as you do not exercise too much of ghc (say ghc-lib-parser strictly being a subset of ghc).

I think there some parts where the two packages diverge, but I do not know for sure. @amesgen might know more details.

Do you mean that a user would do this themselves by manually constraining the version?

(say by passing --constraint="ghc-lib-parser ==9.4.4.* to cabal or alternatively constraining the version through other means like stack or nix)

Yes, this is what I had in mind.

[amesgen]

How is that?
I was under the impression that the packages are interchangeable as long as you do not exercise too much of ghc (say ghc-lib-parser strictly being a subset of ghc).

I think there some parts where the two packages diverge, but I do not know for sure. @amesgen might know more details.

Yeah, differences for the same patch version would probably be a bug, but for different patch versions, they might differ, although I don't think we have ever experienced this so far (concretely: suppose one is compiling with GHC 9.4.1, but ghc-lib-parser has version 9.4.2).

The original point was that

Ormolu might stop being agnostic to whether ghc-lib-parser or ghc in used at some point, e.g. if a change like

which was about a usability concern of downstream users such as HLS, i.e. that was the motivation why hlint changed the default: ndmitchell/hlint#1376

[mrkkrp]

Okay, closing for now.