CtWanted and GHC 8.2.2 in ConCat.BuildDictionary

[conal]

Compile-time warning:

/Users/conal/Haskell/concat/satisfy/src/ConCat/BuildDictionary.hs:140:24: warning: [-Wmissing-fields]
    • Fields of ‘CtWanted’ not initialised: ctev_nosh
    • In the second argument of ‘($)’, namely
        ‘CtWanted
           {ctev_pred = predTy, ctev_dest = EvVarDest evar, ctev_loc = loc}’
      In the expression:
        mkNonCanonical
          $ CtWanted
              {ctev_pred = predTy, ctev_dest = EvVarDest evar, ctev_loc = loc}
      In an equation for ‘nonC’:
          nonC
            = mkNonCanonical
                $ CtWanted
                    {ctev_pred = predTy, ctev_dest = EvVarDest evar, ctev_loc = loc}
    |
140 |                        CtWanted { ctev_pred = predTy
    |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^...

Try a simple example anyway:

ghc: panic! (the 'impossible' happened)
  (GHC version 8.2.2 for x86_64-apple-darwin):
    /Users/conal/Haskell/concat/examples/src/ConCat/BuildDictionary.hs:(140,24)-(142,50): Missing field in record construction ctev_nosh

I don't know why GHC is mis-reporting the file name for ConCat.BuildDictionary

[kosmikus]

This is a new field in CtWanted.

I think @osa1 had this added already in his ghc-8.2 patch. Will look this up.

[conal]

Terrific.

IIRC, the difficulty with @osa1's old ghc-8.2 patch was that it did not also keep the plugin working under 8.0.2, to allow testing and transition. Hopefully we can now get fixes re-added while keeping 8.0.2 working. Sorry for the hassle!

[kosmikus]

You have to change satisfy/src/ConCat/BuildDictionary.hs:

               nonC = mkNonCanonical $
                       CtWanted { ctev_pred = predTy
                                , ctev_dest = EvVarDest evar
#if MIN_VERSION_GLASGOW_HASKELL(8,2,0,0)
                                , ctev_nosh = WOnly
#endif
                                , ctev_loc = loc }

I'm not sure (and it seem @osa1 was neither) about what value to use there. Possible choices are WDeriv and WOnly. The documentation isn't immediately conclusive. I'll ask.

[kosmikus]

No, I think @osa1's patch actually was compiling for all of 8.0, 8.2, and 8.4. I think he was just too perfectionistic and didn't want to submit it as a PR while 8.2 and 8.4 was clearly still broken. And then in the meantime, master had diverged a lot, so it no longer cleanly applied. I've compared his patch and @cartazio's though. Most differences are in the details. This CtWanted is one of the few other 8.2-relevant changes. There are a few more for 8.4.

[conal]

Oh! I'm really sorry we diverged. :( And of course we'll want to be on 8.4 soon. I'm especially eager to get working on 8.6, since that version will allow plugins not to trigger unnecessary recompilations.

I don't have a lot of experience with collaborative programming. If you have advice for me about how not to cause the repo to diverge during our work together, I'm all ears.

[kosmikus]

Well, to a large extent it was us being slow, so our fault.

But one thing that would help a bit is to make sure that master always builds and some minimum amount of tests always pass (via CI), and to do experiments on separate branches and only merge them to master once CI confirms that they don't cause regressions.

[conal]

Sounds sensible. Thanks!

[kosmikus]

Do you want me to submit the ctev_nosh change as a PR, or are you doing it?

[conal]

I'll do it.I'll do it, but I'd appreciate your checking into whether to use WDeriv or WOnly in CtWanted. Meanwhile, I'll to leave this issue open as a reminder.