Skip to content
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

Allow building with template-haskell-2.21.* (GHC 9.8) #1046

Merged
merged 5 commits into from
Aug 24, 2023
Merged

Conversation

RyanGlScott
Copy link
Collaborator

This is made quite simple by using the TyVarBndrVis/bndrVis compatibility shims from th-abstraction-0.6.*. When that version of th-abstraction is not available, I provide these definitions using CPP.

Fixes #1043.

While I was in town, I also made sure that the Prisms that Language.Haskell.TH.Lens offers are up to date with recent versions of the template-haskell library.

This is made quite simple by using the `TyVarBndrVis`/`bndrVis` compatibility
shims from `th-abstraction-0.6.*`. When that version of `th-abstraction` is not
available, I provide these definitions using CPP.

Fixes #1043.
@RyanGlScott
Copy link
Collaborator Author

docspec is failing on the GHC 9.8.1 job, which reveals that there are some uses of head and tail in the doctests which I failed to notice in #1045:

[  20.23108] error: [1,2,3,4] & upon head .~ 0
[  20.23164] error: in comment at 160:1
+
+<interactive>:9088:18: warning: [GHC-63394] [-Wx-partial]
+    In the use of ‘head’
+    (imported from Prelude, but defined in GHC.List):
+    "This is a partial function, it throws an error on empty lists. Use pattern matching or Data.List.uncons instead. Consider refactoring to use Data.List.NonEmpty."
 [0,2,3,4]
[  20.26376] error: upon' (tail.tail) .~ [10,20] $ [1,2,3,4]
[  20.26495] error: in comment at 204:1
+
+<interactive>:9098:8: warning: [GHC-63394] [-Wx-partial]
+    In the use of ‘tail’
+    (imported from Prelude, but defined in GHC.List):
+    "This is a partial function, it throws an error on empty lists. Replace it with drop 1, or use pattern matching or Data.List.uncons instead. Consider refactoring to use Data.List.NonEmpty."
+
+<interactive>:9098:13: warning: [GHC-63394] [-Wx-partial]
+    In the use of ‘tail’
+    (imported from Prelude, but defined in GHC.List):
+    "This is a partial function, it throws an error on empty lists. Replace it with drop 1, or use pattern matching or Data.List.uncons instead. Consider refactoring to use Data.List.NonEmpty."
 [1,2,10,20]
[  20.30274] error: onceUpon (tail.tail) .~ [10,20] $ [1,2,3,4] -- BAD
[  20.30804] error: in comment at 225:1
+
+<interactive>:9108:11: warning: [GHC-63394] [-Wx-partial]
+    In the use of ‘tail’
+    (imported from Prelude, but defined in GHC.List):
+    "This is a partial function, it throws an error on empty lists. Replace it with drop 1, or use pattern matching or Data.List.uncons instead. Consider refactoring to use Data.List.NonEmpty."
+
+<interactive>:9108:16: warning: [GHC-63394] [-Wx-partial]
+    In the use of ‘tail’
+    (imported from Prelude, but defined in GHC.List):
+    "This is a partial function, it throws an error on empty lists. Replace it with drop 1, or use pattern matching or Data.List.uncons instead. Consider refactoring to use Data.List.NonEmpty."
 [1,10,20]

I'll have to think about the best way to adapt these examples.

@RyanGlScott
Copy link
Collaborator Author

I took a look at the Data.Data.List doctests that use head/tail, and remarkably, some of these actually rely on head/tail being partial functions, such as this one:

lens/src/Data/Data/Lens.hs

Lines 181 to 182 in 7267819

-- >>> "" ^? upon tail
-- Nothing

It turns out that there is some grimy code here that catches all exceptions thrown (in pure code!) when invoking upon in order to make this work... wild stuff.

We could certainly define our own custom versions of head and tail to avoid triggering -Wx-partial warnings, but that would arguably make the doctests less useful, since readers would have to figure out what our custom functions are doing. head and tail, dangerous as they may be, are well-known among Haskellers, and using them has the advantage that readers don't have to spend mental energy figuring out what they're meant to do.

For now, I'm inclined to just disable -Wx-partial when running the doctests in Data.Data.Lens. It's possible that we may have to revisit this choice some day (e.g., if a future version of the Prelude removes head/tail altogether), but that is a problem for future me.

This avoids a `-Wx-partial` warning with GHC 9.8.
Some doctests in `Data.Data.Lens` use the partial `head` and `tail` functions,
which now produce `-Wx-partial` warnings (implied by `-Wall`) as of GHC 9.8.
Moreover, some of these doctests _rely_ on `head`/`tail` being partial, so it's
not entirely straightforward to change them. To accommodate this, we redefine
the `head`/`tail` functions (see the new `Control.Lens.Internal.Doctest`
module) so that they can be used in the doctests without triggering any
warnings. The end result should be indistinguishable to readers of the
Haddocks.
@RyanGlScott
Copy link
Collaborator Author

As noted in 850b587#commitcomment-125292040, it's not entirely straightforward to disable a warning only in the doctests, nor is it even guaranteed to work reliably. As such, I've pushed an alternative approach that instead redefines head/tail in an internal module and replaces the use of head/tail from the Prelude (which emit -Wx-partial warnings) with our custom, warning-free versions. Luckily, the end result is that the doctests look exactly the same to readers of the Haddocks.

@RyanGlScott RyanGlScott merged commit dfbd591 into master Aug 24, 2023
22 checks passed
@RyanGlScott RyanGlScott deleted the T1043 branch August 24, 2023 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build failure with template-haskell-2.21 (GHC 9.8)
1 participant