Use GADTs for all ghc versions in Development.IDE.Plugin.Completions.Logic #947

jneira · 2020-12-10T10:31:31Z

It fixes the build for ghc-8.8.3 and ghc-8.8.2 that was failing with:

Progress 166/173: ghcide ghcide > /root/build/ghcide/src/Development/IDE/Plugin/Completions/Logic.hs:267:5: error:
Progress 166/173: ghcide ghcide >     • Illegal equational constraint NoGhcTcPass pass
Progress 166/173: ghcide ghcide >                                     ~ NoGhcTcPass (NoGhcTcPass pass)
Progress 166/173: ghcide ghcide >       (Use GADTs or TypeFamilies to permit this)
Progress 166/173: ghcide ghcide >     • When checking the inferred type
Progress 166/173: ghcide ghcide >         f :: forall (pass :: Pass).
Progress 166/173: ghcide ghcide >              (Outputable.OutputableBndr
Progress 166/173: ghcide ghcide >                 (NameOrRdrName (IdP (GhcPass (NoGhcTcPass pass)))),
Progress 166/173: ghcide ghcide >               Outputable.OutputableBndr (IdP (GhcPass (NoGhcTcPass pass))),
Progress 166/173: ghcide ghcide >               Outputable.OutputableBndr (IdP (GhcPass pass)),
Progress 166/173: ghcide ghcide >               Outputable.OutputableBndr (NameOrRdrName (IdP (GhcPass pass))),
Progress 166/173: ghcide ghcide >               Outputable (XIPBinds (GhcPass pass)),
Progress 166/173: ghcide ghcide >               Outputable (XViaStrategy (GhcPass pass)),
Progress 166/173: ghcide ghcide >               Outputable (XIPBinds (GhcPass (NoGhcTcPass pass))),
Progress 166/173: ghcide ghcide >               Outputable (XViaStrategy (GhcPass (NoGhcTcPass pass))),
Progress 166/173: ghcide ghcide >               NoGhcTcPass pass ~ NoGhcTcPass (NoGhcTcPass pass)) =>
Progress 166/173: ghcide ghcide >              Maybe Range -> ImportDecl (GhcPass pass) -> Maybe [TextEdit]
Progress 166/173: ghcide ghcide >       In the expression:
Progress 166/173: ghcide ghcide >         let
Progress 166/173: ghcide ghcide >           f (Just range) ImportDecl {ideclHiding}
Progress 166/173: ghcide ghcide >             = case ideclHiding of
Progress 166/173: ghcide ghcide >                 Just (False, x)
Progress 166/173: ghcide ghcide >                   | Set.notMember name (Set.fromList ...) -> ...
Progress 166/173: ghcide ghcide >                   | otherwise -> ...
Progress 166/173: ghcide ghcide >                 _ -> ...
Progress 166/173: ghcide ghcide >           f _ _ = Nothing
Progress 166/173: ghcide ghcide >           src_span = srcSpanToRange . getLoc $ lDecl
Progress 166/173: ghcide ghcide >         in f src_span . unLoc $ lDecl
Progress 166/173: ghcide ghcide >       In an equation for ‘extendImportList’:
Progress 166/173: ghcide ghcide >           extendImportList name lDecl
Progress 166/173: ghcide ghcide >             = let
Progress 166/173: ghcide ghcide >                 f (Just range) ImportDecl {ideclHiding} = ...
Progress 166/173: ghcide ghcide >                 f _ _ = Nothing
Progress 166/173: ghcide ghcide >                 src_span = srcSpanToRange . getLoc $ lDecl
Progress 166/173: ghcide ghcide >               in f src_span . unLoc $ lDecl
Progress 166/173: ghcide ghcide >     |
Progress 166/173: ghcide ghcide > 267 |     f (Just range) ImportDecl {ideclHiding} = case ideclHiding of
Progress 166/173: ghcide ghcide >     |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^...
Progress 166/173: ghcide ghcide >

It unblocks the bump of ghcide in hls (and the incoming release): haskell/haskell-language-server#658.
Will rebase #946 when it is ready

ndmitchell

Why not just include the GADTs extension unconditionally? CPP seems significantly worse than a fairly harmless extension.

jneira · 2020-12-10T10:34:53Z

Why not just include the GADTs extension unconditionally? CPP seems significantly worse than a fairly harmless extension.

ghc-8.6 definitely does not need it but we can give a try

ndmitchell · 2020-12-10T10:36:38Z

Yes, it might be not required, but it's not harmful, so might as well simplify and include it.

pepeiborra · 2020-12-10T13:18:46Z

Can we move this to #946? Otherwise, how do we know that it works?

jneira · 2020-12-10T13:44:42Z

Can we move this to #946? Otherwise, how do we know that it works?

my plan is rebase master after merging #946 in master
(it has worked for me locally though)

jneira · 2020-12-10T15:26:44Z

well in fact we should rebase #946 onto this one, to make ci pass

pepeiborra · 2020-12-10T18:10:45Z

No, we should merge them both :)

jneira · 2020-12-10T18:23:24Z

ok, will include it here

jneira · 2020-12-10T18:28:30Z

dicho y hecho

jneira · 2020-12-10T19:28:05Z

the ghc-lib build had test:true by default

pepeiborra

Thanks!

jneira · 2020-12-10T21:04:02Z

Too much jobs, the matrix dont "overwrite" major versions with test enabled but create new ones, investigating

jneira · 2020-12-11T07:51:07Z

After further investigation in my repo:

include and exclude dont support lists like the matrix so you have to write all combinations one by one 😞
we could make two separate jobs but it would duplicate allmost all steps
- it seems the unique current way to share steps between jobs is create a dedicate github action 😞
we have several axis and corner cases:
- os, ghc version, testability, using ghc-lib
  - we have three versions to test and four versions to only build
- windows only supports some of those ghc versions (to build and to test) and has a specific one (ghc-8.10.2.2) that can be testable

So the unique solution i can think of is create a matrix with all combinations (except ghc-lib and windows ghc-8.10.2.2) and exclude one by one the other ones. Or create a minimal matrix over test (3 versions) or only build (4 versions) without windows and include all desired cases one by one.
In both cases the list of inclusions or exclusions will be annoying. 😟

I am gonna start to sketch the second option (minimal matrix build over only build + inclusions)

jneira · 2020-12-11T09:21:09Z

.github/workflows/test.yml

-          - os: windows-latest
-            ghc: '8.10.2' # broken due to https://gitlab.haskell.org/ghc/ghc/-/issues/18550
-          - os: windows-latest
-            ghc: '8.8.4' # also fails due to segfault :(
        include:


this is the less bad version i got to work, feel free to suggest another better one 🙂
//cc @pepeiborra @ndmitchell

jneira · 2020-12-11T09:44:37Z

I've opened a issue in github actions to let use lists in matrix include and exclude props: actions/runner#857

jneira · 2020-12-11T11:25:57Z

@pepeiborra Unfortunately we already released ghcide-0.6 in hackage (but no in github afaics, only exists the tag)
Could we release 0.6.1 with this merged, to use it in hls?

EDIT: I've just make the github 0.6.0 release, using the pr list from ChangeLog.md

pepeiborra · 2020-12-11T11:34:13Z

I don't have any objections against a 0.6.0.1 release in Hackage. But why is this needed though?

hls-plugin-api need only depend on ghcide 0.6.0
HLS too, but it doesn't get released to Hackage anyway
We can make a Hackage revision for ghcide 0.6.0 that lists it as incompatible with ghc 8.8.2 and 8.8.3

pepeiborra · 2020-12-11T11:40:22Z

EDIT: I've just make the github 0.6.0 release, using the pr list from ChangeLog.md

I don't see the value in GitHub releases so I don't bother with them

jneira · 2020-12-11T12:11:57Z

I don't have any objections against a 0.6.0.1 release in Hackage. But why is this needed though?

If someone wants to build its own version of hls without the ghcide submodule, or a project (a standalone private plugin?) that uses ghcide or hls-plugin-api from hackage, with ghc-8.8.3 or ghc-8.8.2, it will fail.

Not sure if that case exists in the wild but it is possible and it is nicer for those users being able to use its current ghc version instead warn them in the package metadata that they can't.

But we can wait for someone asking for it, so as you consider.

I don't see the value in GitHub releases so I don't bother with them

Well they are merely informative but we did releases for previous versions, so at least for consistency.

…8.3 and ghc-8.8.2 builds (haskell/ghcide#947) * Extend CI matrix with all the GHC minor versions supported by HLS * Adding a new job for windows: ghc-8.10.2.2 * Use GADTs for all ghc versions in Development.IDE.Plugin.Completions.Logic * Fix ghc-8.8.2 and ghc-8.8.3 builds Co-authored-by: Pepe Iborra <[email protected]>

pepeiborra added 2 commits December 10, 2020 08:36

Extend CI matrix with all the GHC minor versions supported by HLS

c13b53b

Run CI actions only on PR, not push

5c9f2e9

jneira requested a review from pepeiborra December 10, 2020 10:31

jneira changed the title ~~Use GADTs for all ghc-8.x versions~~ Use GADTs for all ghc-8.x versions in Development.IDE.Plugin.Completions.Logic Dec 10, 2020

ndmitchell approved these changes Dec 10, 2020

View reviewed changes

jneira mentioned this pull request Dec 10, 2020

Prepare 0.7.0 release (includes bump ghcide to 0.6.0.1) haskell/haskell-language-server#658

Merged

jneira force-pushed the fix-ghc-8.x branch from 7772f7b to 730e90d Compare December 10, 2020 10:38

jneira changed the title ~~Use GADTs for all ghc-8.x versions in Development.IDE.Plugin.Completions.Logic~~ Use GADTs for all ghc versions in Development.IDE.Plugin.Completions.Logic Dec 10, 2020

jneira added 2 commits December 10, 2020 13:21

Include os's in all include blocks

b788d8b

Fix include blocks

fca6e9d

Use GADTs for all ghc versions

0690fb7

jneira force-pushed the fix-ghc-8.x branch from 730e90d to 0690fb7 Compare December 10, 2020 18:27

jneira mentioned this pull request Dec 10, 2020

Extend CI matrix with all the GHC minor versions supported by HLS #946

Closed

pepeiborra approved these changes Dec 10, 2020

View reviewed changes

jneira added 2 commits December 11, 2020 09:10

Reorganize matrix to reduce jobs

69aaeeb

Add comments and fix test step if

695fc48

jneira force-pushed the fix-ghc-8.x branch from 46b1719 to 695fc48 Compare December 11, 2020 09:19

jneira commented Dec 11, 2020

View reviewed changes

jneira merged commit 27b4250 into master Dec 11, 2020

jneira deleted the fix-ghc-8.x branch December 11, 2020 11:23

Use GADTs for all ghc versions in Development.IDE.Plugin.Completions.Logic #947

Use GADTs for all ghc versions in Development.IDE.Plugin.Completions.Logic #947

Uh oh!

Conversation

jneira commented Dec 10, 2020

Uh oh!

ndmitchell left a comment

Choose a reason for hiding this comment

Uh oh!

jneira commented Dec 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ndmitchell commented Dec 10, 2020

Uh oh!

pepeiborra commented Dec 10, 2020

Uh oh!

jneira commented Dec 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jneira commented Dec 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pepeiborra commented Dec 10, 2020

Uh oh!

jneira commented Dec 10, 2020

Uh oh!

jneira commented Dec 10, 2020

Uh oh!

jneira commented Dec 10, 2020

Uh oh!

pepeiborra left a comment

Choose a reason for hiding this comment

Uh oh!

jneira commented Dec 10, 2020

Uh oh!

jneira commented Dec 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jneira Dec 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jneira commented Dec 11, 2020

Uh oh!

jneira commented Dec 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pepeiborra commented Dec 11, 2020

Uh oh!

pepeiborra commented Dec 11, 2020

Uh oh!

jneira commented Dec 11, 2020

Uh oh!

Uh oh!

jneira commented Dec 10, 2020 •

edited

Loading

jneira commented Dec 10, 2020 •

edited

Loading

jneira commented Dec 10, 2020 •

edited

Loading

jneira commented Dec 11, 2020 •

edited

Loading

jneira Dec 11, 2020 •

edited

Loading

jneira commented Dec 11, 2020 •

edited

Loading