Consider defaulting to ghc-lib

[pepeiborra]

The Cabal flag ghc-lib currently defaults to False. Consider changing the default value to True so that projects like HLS that depend on hlint can have a working cabal install ... command across all GHC versions.

[ndmitchell]

There were two reasons for doing it that way:

• For compatible GHC versions, it allows HLS to pass the AST from GHC along without modification, which means less memory, less parsing time, and great compatibility. Is that no longer desired? Or do you have ways to get those advantages without using the same one as the GHC API?
• It makes it slower to compile for users - quite significantly slower. But if it's easier for HLS users, that's probably a worthwhile tradeoff, given there are provided binaries.

[pepeiborra]

There were two reasons for doing it that way:

• For compatible GHC versions, it allows HLS to pass the AST from GHC along without modification, which means less memory, less parsing time, and great compatibility. Is that no longer desired?

Less memory and parsing time, yes, but why do you say greater compatibility?

Or do you have ways to get those advantages without using the same one as the GHC API?

No, but we are willing to give them up in exchange for greater compatibility. Manual Cabal flags have a poor user interface, specially in combination with Hackage.

• It makes it slower to compile for users - quite significantly slower. But if it's easier for HLS users, that's probably a worthwhile tradeoff, given there are provided binaries.

Ah, this is a good point.

Summarising, HLS really wants to depend on the ghc-lib version of hlint, because that one works on all ghc versions and HLS already depends on ghc-lib anyway. Or as we do now, selectively use the ghc-lib hlint on non-native ghc versions, but in a declarative way.

I can see how defaulting the flag to true would be a net loss for standalone hlint use cases.

It occurs to me that the best solution would be a separate package hlint-ghclib.

[ndmitchell]

but why do you say greater compatibility?

You get a AST out of GHC, that must have had all the right CPP, extensions etc set, because you are using it for compilation. The worry is that when HLS tries to create a second GHC AST, it might get some things wrong. This is a worry, but one careful coding can get around.

No, but we are willing to give them up in exchange for greater compatibility.

Cool, that sounds a reasonable trade-off to make.

Manual Cabal flags have a poor user interface, specially in combination with Hackage.

Agreed!

As to how to proceed, moving to ghc-lib by default will slow down compilation times for GHC 9.2.2 users, but in every other way makes things simpler. Two packages would be a bit of a pain, probably not worth the hassle. So it sounds like default to ghc-lib is reasonable and better for HLS.

CC @shayne-fletcher - is there anything you can think that would go wrong if we defaulted to ghc-lib? I know there are some upstream ghc-lib flags that we have to adjust to make things compatible sometimes, but don't think any impact here?

[shayne-fletcher]

this blog is what i use to remind myself how all this works.

hlint & ghc-lib-parser-ex need to be kept in sync (they both depend on one of ghc-lib-parser or ghc and it needs to be the same choice for both or havoc results).

• hlint.cabal defaults the flag ghc-lib to False

  • default behavior is:
    • build depend on ghc if the build compiler is ghc-9.2.2
    • build depend on ghc-lib-parser-9.2.* otherwise
    • in either case build depend on ghc-lib-parser-ex

• ghc-lib-parser-ex.cabal defaults the flag auto to True

  • default behavior is:
    • build depend on ghc if the build compiler is ghc-9.2.2
    • build depend on ghc-lib-parser-9.2.* otherwise

if hlint.cabal changes the ghc-lib flag to True we'd neccessarily change ghc-lib-parser-ex.cabal flag auto to False at the same time.

[shayne-fletcher]

i think this is a reasonable change and would not object to us making it.

[shayne-fletcher]

see also shayne-fletcher/ghc-lib-parser-ex#106 (comment). @phadej would you view this change positively too?

[shayne-fletcher]

@pepeiborra @ndmitchell are we agreed on this?

if you indicate consent i propose to release a major version of ghc-lib-parser-ex with the change and following that send a PR to hlint to use it.

[ndmitchell]

Sounds good - go for it!

[ndmitchell]

So I see we have both #1385 (flipping the default to True) and #1378 (adding a subcomponent). @pepeiborra - does the subcomponent make it easier for HSE, even if we just flip the default to always be True? Are there other benefits it provides, or is this an either/or solution? And if it's a choice, which would you prefer?

[pepeiborra]

The sub component works equally well for HSE (assuming we can work out the syntax to use it).
The main benefit of the sub component approach is preserving the existing hlint binary that does not use ghc-lib.
But there are downsides too, they are not handled well by stack repl: commercialhaskell/stack#4564

[pepeiborra]

If we flip the default value to yes ghc-lib, then the sub component adds nothing for HLS

[ndmitchell]

Given that new Cabal features tend to have fallout no one can expect (I've been burnt before) I'm inclined to move to ghc-lib on all versions. But alas, that will require waiting for GHC 9.2.3 to have been out a bit longer (a few things still default to 9.2.2) as the Mac build otherwise breaks. I'm envisaging about a month. I guess given we've lived with the issue for a long time, another month seems ok?

[lf-]

I found one such bug: haskell/haskell-language-server#3095

[shayne-fletcher]

fixed via #1410

[shayne-fletcher]

I found one such bug: haskell/haskell-language-server#3095

hi @lf- could you clarify please? what is meant by "one such bug"?

[lf-]

Sorry, I was unclear. I meant a bug related to reparsing the files in hlint with different flags compared to how they were parsed in hls.