Closed sellout closed 1 year ago
@sellout Is this patch good to go in? Or what's the right order to do this in wrt. #106 ?
It could go in before #106 is dealt with, but the problem there is that the GHC 9 support either won't be tested, or we'll need to add some logic to CI to tell it not to test concat-plugin (but still test concat-classes, concat-examples, etc.) on GHC 9.
I'm fine with any of the three options (wait for #106, merge without testing GHC 9, or add GHC 9 testing logic). Both #91 (merged) and #106 were motivated by this work, though.
FWIW, I've started working on this.
So test:misc-examples
and the test:gold-tests
work now. WIP is here:
https://github.com/compiling-to-categories/concat/tree/ghc-9
This is tested only on ghc 9.2.4. I guess the next step is to cherry-pick the relevant changes back to master and then unbreak the previous versions again.
One question, for @conal and @sellout: How far back should support ghc versions? I'm thinking 8.10.7 is the earliest still realistically viable.
So
test:misc-examples
and thetest:gold-tests
work now.
Great!
One question, for @conal and @sellout: How far back should support ghc versions? I'm thinking 8.10.7 is the earliest still realistically viable.
Sounds fine to me, but I think @sellout will be more affected than I am by the choice.
8.10.7 is fine by me – my coworkers already get frustrated by my need to support as many versions as possible, so this will make everyone happy 😆
So the ghc-9
branch is done now, AFAIC.
@sellout: My idea would be to just squash all the commits into one on top of master
. Would that be OK? (Mentioning you in the commit message of course.)
Yeah, go ahead and squash. Just add Co-authored-by: Greg Pfeil <greg@technomadic.org>
to the commit message.
Thanks for getting this done!
Done that. Thanks!
Mostly trivial changes, but we now get the
DynFlags
frominstallCoreToDos
rather than each application ofBuiltinRule.ru_try
.This continues to work just fine for GHC 8.10, but doesn't quite work on GHC 9.0 for some reason. I don't think it's the
DynFlags
changes (as that's applied to all GHC versions). But running on GHC 9.0 always fails with "isDeadEndId cccV" (this is slightly stricter thanisBottomingId
in earlier GHCs, but removing thatpprPanic
still fails, just with "Oops: toCcc'' called" instead).Fixes #98.