-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update for GHC 9.8 #547
Update for GHC 9.8 #547
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. LGTM, with a question about NE.fromList
.
Text/Megaparsec/Char/Lexer.hs
Outdated
@@ -346,7 +346,7 @@ charLiteral = label "literal character" $ do | |||
r <- lookAhead (count' 1 10 anySingle) | |||
case listToMaybe (Char.readLitChar r) of | |||
Just (c, r') -> c <$ skipCount (length r - length r') anySingle | |||
Nothing -> unexpected (Tokens (head r :| [])) | |||
Nothing -> unexpected (Tokens (NE.fromList (take 1 r))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't a use of NE.fromList
as bad as using head
? Both will throw the exception in the library, not in the application code.
If all that head is bad excitement should lead to improvement, then the new code should be fromMaybe undefined $ NE.nonEmpty $ take 1 r
which will raise the error (via undefined
) here, not in the library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both head
and NE.fromList
use HasCallStack
, so they will at least point to the code here. But yeah, I don't think there is any big advantage of using NE.fromList
rather than head
other than to avoid the warning (in case that is even desired).
I published a revision of the bytestring bound to hackage: https://hackage.haskell.org/package/megaparsec-9.5.0/revisions/ |
- 9.6.3 fixed the false positives with `-Wredundant-constraints` - `haskell/actions/setup` got deprecated in favor of `haskell-actions/setup`
Thanks! |
I will make a revision on Hackage. |
Otherwise, one might get errors like
as
bytestring-0.12
(shipped with GHC 9.8) is non-reinstallable when using TH.Allowing
containers-0.7
is not strictly necessary, but it compiles fine.Due to this change in
base-4.19
there are various new warnings with GHC 9.8. We fix these by enabling
-Wno-x-partial
.