Support GHC 9.8.1 #1310
Support GHC 9.8.1 #1310
Conversation
larskuhtz
requested review from
emilypi,
jmcardon and
jwiegley
as code owners
larskuhtz
force-pushed
the
lars/ghc-9.8.1
branch
from
1003ba8 to
17f5450
Compare
|
CI seems to be broken with that last one @larskuhtz |
|
Yeah, not sure why the nix builds fail. The failure message is a bit cryptic. Some nix expert has to take a look. |
This reverts commit a95e012.
|
The nix m1 build now complains about x-partial (which comes as a surprise, because AFAIK this PR did not upgrade the nix build to use 9.8.). Also not sure about the other nix builds. The failure message are not meaningful (to me). |
|
I also reverted webauthn 0.8 support from the PR, since webauthn it is now vendored. |
|
@imalsogreg can you take another look at the nix builds? I don't know how to fix them. |
| _:xs -> xs | ||
| [] -> error "Expected nonempty list" |
There was a problem hiding this comment.
I'm curious if this can be changed to avoid the preceding null protoSteps check, and returning an empty list in this branch instead of erroring out? That is, smth like
(case reverse protoSteps of
[] -> []
_:xs -> xs)| let i = if null vs then info else _tInfo $ view _1 $ head vs | ||
| CyclicSCC [] -> error "Expected nonempty list" | ||
| CyclicSCC vs@(v:_) -> do | ||
| let i = if null vs then info else _tInfo $ view _1 $ v |
There was a problem hiding this comment.
null vs shall always be False here, right? Also, I think you can drop this last $ before v.
There was a problem hiding this comment.
Yeah, need to drop the if null if the above change is to make it in.
| AcyclicSCC v -> return v | ||
| CyclicSCC vs -> do | ||
| let i = if null vs then info else _tInfo $ view _1 $ head vs | ||
| CyclicSCC [] -> error "Expected nonempty list" |
There was a problem hiding this comment.
The previous impl proceeded as usual binding i to info in this case if my brain is symbolic-executing the code right. Although I'm not sure if that makes sense given the semantics of this branch.
There was a problem hiding this comment.
evalError or something else here, even if unreachable, just in case.
| [TxLog "USER_user1" "key1" row, | ||
| TxLog "USER_user1" "key1" row'] $ | ||
| _getTxLog pactdb usert (head tids) v | ||
| case tids of |
There was a problem hiding this comment.
Shall we still check assertEquals "user txids" [1] tids?
| -- | unsafe lens for using `typecheckBody` with const | ||
| singLens :: Iso' a [a] | ||
| singLens = iso pure head | ||
| singLens = iso pure (\case |
There was a problem hiding this comment.
I'd add HasCallStack constraint to this function to get better traces where it happened for debuggability (unless it's a hot spot, though I doubt it is).
| allow-newer: *:text | ||
| allow-newer: *:bytestring | ||
|
|
||
| -- Patch merged into master (upcoming verison 10.0). We are currently using 9.2 |
| final.haskell-nix.project' { | ||
| src = ./.; | ||
| compiler-nix-name = "ghc962"; | ||
| compiler-nix-name = "ghc981"; |
There was a problem hiding this comment.
This seems to be why this PR is making nix use 9.8.
There was a problem hiding this comment.
@larskuhtz Were we aiming to use ghc 9.8.1 in this PR? I thought so, but let me check my assumption.
There was a problem hiding this comment.
On the cabal build side, with PR there are builds for both, ghc-9.6 and ghc-9.8.
Not sure what we want on the nix side. Personally, I'd say that in case of doubt we should prefer the latest.
| AcyclicSCC v -> return v | ||
| CyclicSCC vs -> do | ||
| let i = if null vs then info else _tInfo $ view _1 $ head vs | ||
| CyclicSCC [] -> error "Expected nonempty list" |
There was a problem hiding this comment.
evalError or something else here, even if unreachable, just in case.
| let i = if null vs then info else _tInfo $ view _1 $ head vs | ||
| CyclicSCC [] -> error "Expected nonempty list" | ||
| CyclicSCC vs@(v:_) -> do | ||
| let i = if null vs then info else _tInfo $ view _1 $ v |
There was a problem hiding this comment.
Yeah, need to drop the if null if the above change is to make it in.
| [TxLog "USER_user1" "key1" row, | ||
| TxLog "USER_user1" "key1" row'] $ | ||
| _getTxLog pactdb usert tid v | ||
| _ -> error "Expected nonempty list of tids" |
There was a problem hiding this comment.
Isn't there an assertFailure or something like this instead of error here?
Uh oh!
There was an error while loading. Please reload this page.