WIP: GHC-9.0 support for hls-tactics-plugin

[anka-213]

It compiles and most tests succeed, but some fail. In particular, the ones where it should suggest show fails to find that as a possible solution and fails to find evidence for Show a.

[anka-213]

My attempt at analysing the issue with test/golden/GoldenShow.hs:

Adding a few orphan instances for Show for a few GHC types

deriving instance Show (HsBindLR GhcTc GhcTc)
deriving instance Show (MatchGroup GhcTc (LHsExpr GhcTc))
deriving instance Show Origin
deriving instance Show NoExtCon
deriving instance Show NoExtField
deriving instance Show MatchGroupTc
deriving instance Show NPatBindTc
deriving instance Show TcEvBinds
instance Show EvBindsVar
    where show _ = "<EvBindsVar>"
deriving instance Show EvBind
instance Show EvTerm
    where show _ = "<EvTerm>"
instance Show EvExpr
    where show _ = "<EvExpr>"

#if __GLASGOW_HASKELL__ >= 900
instance Show (Scaled Type)
    where show x = "Scaled _ " ++ show (scaledThing x)
#endif
instance Show (GRHSs GhcTc (LHsExpr GhcTc))
    where show _ = "<GRHSs GhcTc (LHsExpr GhcTc)>"
deriving instance Show (PatSynBind GhcTc GhcTc)
deriving instance Show (HsPatSynDetails (Located Id))
instance Show NameSet
    where show _ = "<tickish>"
deriving instance Show (RecordPatSynField (Located Id))
deriving instance Show (HsPatSynDir GhcTc)
instance Show (CoreSyn.Tickish Id)
    where show _ = "<tickish>"
instance Show (ABExport GhcTc)
    where show _ = "<ABExport GhcTc>"
instance Show HsWrapper
    where show _ = "<HsWrapper>"

and adding a bit of debug printing (here):

absBinds dst (L src abnd@(AbsBinds _ _ h _ _ _ _))
  | dst `isSubspanOf` src = traceX "absBinds" abnd $ fmap idType h

and making sure that the debug printing is actually visible (by changing here)

-runSessionWithServer' plugin conf sconf caps root s = withLock lock $ keepCurrentDirectory $ silenceStderr $ do
+runSessionWithServer' plugin conf sconf caps root s = withLock lock $ keepCurrentDirectory $ do

tells us that we never got any evidence for Show from GHC for ghc-9.0.1

AbsBinds {abs_ext = NoExtField, abs_tvs = [], abs_ev_vars = [], abs_exports = [<ABExport GhcTc>], abs_ev_binds = [EvBinds []], abs_binds = [showMe = __aLN], abs_sig = True}

but we did get that evidence for ghc-8.10.7

AbsBinds {abs_ext = NoExtField, abs_tvs = [a], abs_ev_vars = [$dShow_aOL], abs_exports = [<ABExport GhcTc>], abs_ev_binds = [EvBinds [EvBind {eb_lhs = __aOO, eb_rhs = <EvTerm>, eb_is_given = False}]], abs_binds = [showMe = __aOO], abs_sig = True}

I don't know enough about the internals of GHC nor about what has changed since ghc-8.x.x to make an educated guess about what caused this difference.

Does @isovector know anything more?

[isovector]

THANK YOU SO MUCH for this PR. I've been dreading digging through the API changes. I really really appreciate you having looked into this, @anka-213.

As for the regression, I'm not sure. I found that stuff originally by showAstDataing my way through the AST and looking for any place evidence would come up. Maybe someone more familiar with GHC9 would know?

[anka-213]

The main method I've been using is iterating through:

• 1. Compiling and looking at type errors
• 2a. If the change is obvious, make that change
• 2b. If it's not, running git -pS 'nameOfRelevantFunction' on the GHC repo and reading the commit message and changes to that function. If that is not enough, look at how uses of that function changed to see if I can replicate that change.
  And then adding CPP pragmas to make older versions compile

But yes, this process is definitely tedious at times, so I can see why you've been dreading it. I'm glad that my help is appreciated. 😊

[isovector]

@anka-213 is GoldenShow the only test that's failing?

[anka-213]

@isovector There are three show related tests that fail:

      GoldenShow (golden):                                                            FAIL (28.83s)
        expected: "showMe :: Show a => a -> String\nshowMe = show\n"
         but got: "showMe :: Show a => a -> String\nshowMe _ = \"\"\n"
      GoldenShowCompose (golden):                                                     FAIL (28.83s)
        expected: "showCompose :: Show a => (b -> a) -> b -> String\nshowCompose fba = show . fba\n"
         but got: "showCompose :: Show a => (b -> a) -> b -> String\nshowCompose _ _ = \"\"\n"
      GoldenShowMapChar (golden):                                                     FAIL (28.79s)
        expected: "test :: Show a => a -> (String -> b) -> b\ntest a f = f (show a)\n"
         but got: "test :: Show a => a -> (String -> b) -> b\ntest _ f = f \"\"\n"

other than that, there's a bunch of tests that fail with this:

      GoldenSuperclass (golden):                                                      FAIL (36.50s)
        uncaught exception: SessionException
        Timed out waiting to receive a message from the server.
        Last message received:
        {
            "result": null,
            "jsonrpc": "2.0",
            "id": 2
        }
        

which I am not sure if it's related to some flakiness from me running it locally or if it's an actual bug.

[anka-213]

Oh, and I noticed there's this as well, which is most likely a bug: No, that's just the lack of show again.
Edit again: Or not, since it fails to even give a code action at all, so that's more severe. But it would still fail to produce show even if the first step had succeeded.

      AutoThetaRankN (golden):                                                        FAIL (54.26s)
        uncaught exception: SessionException
        Received an unexpected message from the server:
        Was parsing: Pattern match failure in do expression at test/Utils.hs:112:7-47
        But the last message received was:
        {
            "result": [],
            "jsonrpc": "2.0",
            "id": 1
        }

[anka-213]

I also get this, which I think might be a ghc related issue I've gotten elsewhere as well, so that may well just be a problem with my setup. which fails with the same message in CI.

      MetaBegin (golden):                                                             FAIL (31.52s)
        uncaught exception: SessionException
        Timed out waiting to receive a message from the server.
        Last message received:
        {
            "error": {
                "code": -32603,
                "message": "Internal error: [parse error on input â]"
            },
            "jsonrpc": "2.0",
            "id": 2
        }
      MetaBeginNoWildify (golden):                                                    FAIL (36.56s)
        uncaught exception: SessionException
        Timed out waiting to receive a message from the server.
        Last message received:
        {
            "error": {
                "code": -32603,
                "message": "Internal error: [parse error on input â]"
            },
            "jsonrpc": "2.0",
            "id": 2
        }

[anka-213]

And finally I get this, which seems very minor and not like an actual change in behaviour, just a change in hole numbering:

      UseConLeft (golden):                                                            FAIL (31.08s)
        expected: "test :: Either a b\ntest = Left _w0\n\n"
         but got: "test :: Either a b\ntest = Left _w2\n\n"
      UseConRight (golden):                                                           FAIL (31.10s)
        expected: "test :: Either a b\ntest = Right _w0\n\n"
         but got: "test :: Either a b\ntest = Right _w3\n\n"

[anka-213]

It seems like most of the tests that failed for me also tests on CI: https://github.com/haskell/haskell-language-server/runs/3638594399?check_suite_focus=true

[anka-213]

Looking through the test results again, it looks like all of these:

tactics
  CodeAction.Auto
    golden
      GoldenShow (golden):              FAIL (0.75s)
      GoldenShowCompose (golden):       FAIL (0.78s)
      GoldenShowMapChar (golden):       FAIL (1.46s)
      GoldenSuperclass (golden):        FAIL (5.72s)
      FmapBoth (golden):                FAIL (5.65s)
      FmapJoin (golden):                FAIL (5.64s)
      Fgmap (golden):                   FAIL (5.65s)
      FmapJoinInLet (golden):           FAIL (5.66s)
      AutoForallClassMethod (golden):   FAIL (5.65s)
   theta
      AutoThetaEqCtx (golden):          FAIL (5.64s)

are caused by the same bug where wingman doesn't get evidence for the constraints in context.

Not sure why so many of them fails with a timeout, given that they all almost immediately gives a "Wingman couldn't find a solution" when running manually in an editor. I've improved the error messages for that in my latest commit. Old message:

        uncaught exception: SessionException
        Timed out waiting to receive a message from the server.
        Last message received:
        {
            "result": null,
            "jsonrpc": "2.0",
            "id": 2
        }

Now it instead says

      AutoThetaEqCtx (golden):        FAIL (39.12s)
        Expected WorkspaceApplyEdit.
        Instead got message:
            "Wingman couldn't find a solution"

---

We also have

tactics
  CodeAction.Auto
    theta
      AutoThetaRankN (golden):          FAIL (0.62s)
        Was parsing: Pattern match failure in do expression at test/Utils.hs:112:7-47

where wingman doesn't produce code actions at all

And there's this:

  CodeAction.RunMetaprogram
    beginMetaprogram
      MetaBegin (golden):                                                             FAIL (52.56s)
        uncaught exception: SessionException
        Timed out waiting to receive a message from the server.
        Last message received:
        {
            "error": {
                "code": -32603,
                "message": "Internal error: [parse error on input â]"
            },
            "jsonrpc": "2.0",
            "id": 2
        }
      MetaBeginNoWildify (golden):                                                    FAIL (61.50s)
        uncaught exception: SessionException
        Timed out waiting to receive a message from the server.
        Last message received:
        {
            "error": {
                "code": -32603,
                "message": "Internal error: [parse error on input â]"
            },
            "jsonrpc": "2.0",
            "id": 2
        }

and finally we have a few flakey tests that fails with timeout or succeeds randomly or depending on other factors.

In particular, I believe these failures:

      UseConLeft (golden):                                                            FAIL (31.08s)
        expected: "test :: Either a b\ntest = Left _w0\n\n"
         but got: "test :: Either a b\ntest = Left _w2\n\n"
      UseConRight (golden):                                                           FAIL (31.10s)
        expected: "test :: Either a b\ntest = Right _w0\n\n"
         but got: "test :: Either a b\ntest = Right _w3\n\n"

are caused by all the tests being run in the same process now and that number is stored in some global variable. If you run the tests 3 times with --retry like CI does, all of them will succeed, since the first such test will always have the expected number.

[anka-213]

Perhaps we could mark these tests as broken on ghc9 for now and open new issues to fix them later? Partial support is better than no support at all.

[isovector]

@anka-213 that sounds good to me

[fendor]

Did this cause any issues or something? I would prefer to have as few CPP statements as possible, as you lose very quickly the big picture about what happens in Core.hs

[anka-213]

Oh, I see. Yeah, that should be fairly easy to workaround. I was trying to avoid having redundant

foo = Module.foo

definitions and just reexporting the existing things. I think removing these pragmas would just cause a few "duplicate exports" warnings, so another option could be to just disable that warning in this file.

Edit: The primary reason why is that I wanted to re-export the whole module, which is what caused the "duplicate exports" warnings. Getting rid of duplicate definitions was just a bonus.

[jneira]

Perhaps we could mark these tests as broken on ghc9 for now and open new issues to fix them later? Partial support is better than no support at all.

agree, not sure if we could get that ready for this month release

[anka-213]

Should I try completing the tasks quickly or is it better to take it slowly an aim for it to be included in next month's release instead?

[jneira]

Should I try completing the tasks quickly or is it better to take it slowly an aim for it to be included in next month's release instead?

As you consider, of course. There is no special rush to have it, so the slow path sounds good.

[isovector]

I'd like to get this up and running. Interested in a hand on it, @anka-213 ?

[anka-213]

Yes please!

[Ailrun]

@anka-213, @isovector Is there any update?

[isovector]

@Ailrun no updates. #2448 means that even if this were working in GHC 9 it still wouldn't work.

[anka-213]

I'll have more time to look at this now than I've had the last few weeks.

[isovector]

I started looking into this. The show tests are failing because Wingman is failing to pull show out of the theta.

[isovector]

My assumption is that theta evidence is stored somewhere different in the tcs_binds as of GHC 9. Here's a new minimal dump:

{Bag(Located (HsBind Var)):
 [({ ss }
   (VarBind
    (NoExtField)
    {Var: $trModule}
    ({ ss }
     (HsApp
      (NoExtField)
      ({ ss }
       (HsApp
        (NoExtField)
        ({ ss }
         (HsConLikeOut
          (NoExtField)
          ({abstract:ConLike})))
        ({ ss }
         (HsPar
          (NoExtField)
          ({ ss }
           (HsApp
            (NoExtField)
            ({ ss }
             (HsConLikeOut
              (NoExtField)
              ({abstract:ConLike})))
            ({ ss }
             (HsLit
              (NoExtField)
              (HsStringPrim
               (NoSourceText)
               "main")))))))))
      ({ ss }
       (HsPar
        (NoExtField)
        ({ ss }
         (HsApp
          (NoExtField)
          ({ ss }
           (HsConLikeOut
            (NoExtField)
            ({abstract:ConLike})))
          ({ ss }
           (HsLit
            (NoExtField)
            (HsStringPrim
             (NoSourceText)
             "Main")))))))))))
 ,({ ss }
   (AbsBinds
    (NoExtField)
    []
    []
    [(ABE
      (NoExtField)
      {Var: showMe}
      {Var: showMe}
      (WpHole)
      (SpecPrags
       []))]
    [({abstract:TcEvBinds})]
    {Bag(Located (HsBind Var)):
     [({ ss }
       (FunBind
        (WpCompose
         (WpCompose
          (WpTyLam
           {Var: a})
          (WpEvLam
           {Var: $dShow_a2pa}))
         (WpLet
          ({abstract:TcEvBinds})))
        ({ ss }
         {Var: showMe})
        (MG
         (MatchGroupTc
          []
          (FunTy
           (VisArg)
           (TyConApp
            ({abstract:TyCon})
            [])
           (TyVarTy
            {Var: a})
           (TyConApp
            ({abstract:TyCon})
            [])))
         ({ ss }
          [({ ss }
            (Match
             (NoExtField)
             (FunRhs
              ({ ss }
               {Name: showMe})
              (Prefix)
              (NoSrcStrict))
             []
             (GRHSs
              (NoExtField)
              [({ ss }
                (GRHS
                 (NoExtField)
                 []
                 ({ ss }
                  (HsVar
                   (NoExtField)
                   ({ ss }
                    {Var: __a2pd})))))]
              ({ ss }
               (EmptyLocalBinds
                (NoExtField))))))])
         (FromSource))
        []))]}
    (True)))]}

[isovector]

Looks like the evidence is now inside of the FunBind of the AbsBinds.

[isovector]

I've got the theta issues worked out. Now the tests that are failing are:

• MetaBeginNoWildify
• MetaBegin
• AutoThetaRankN
• FmapJoinInLet
• FmapJoin

None are failing due to bad output, but to crashing the server (or flakey tests).

[isovector]

AutoThetaRankN fails because the code actions don't appear on the hole. Fixed if I eta expand the hole.

MetaBegin/MetaBeginNoWildify fail with Internal error: [parser error on input '||]']. Probably some jankiness related to exactprint.

FmapJoin and FmapJoinInLet work, but are slow af and probably timing out the test.

[isovector]

So the problem with BeginMeta is not exactprint --- it's correctly producing the program

begin = [wingman| |]

but GHC is failing to parse this. Looks like I need to turn on TH syntax in the dflags?

[jneira]

I think we can close this and focus in the new one, thanks @anka-213