sjshuck / hs-pcre2

Complete Haskell binding to PCRE2
Apache License 2.0
12 stars 2 forks source link

future-proof for simplified subsumption in GHC 9.0 #16

Closed amesgen closed 3 years ago

amesgen commented 3 years ago

https://gitlab.haskell.org/ghc/ghc/-/wikis/migration/9.0#simplified-subsumption

Another option would be to eta-expand the Traversal's.

To test with stack, use https://gist.github.com/tfausak/a7ef9af57a9f0c0099f187cd3d920a87

Compiler errors on GHC 9.0 without this change ``` src/hs/Text/Regex/Pcre2/Internal.hs:1289:16: error: • Couldn't match type: (NonEmpty Text -> f0 (NonEmpty Text)) -> Text -> f0 Text with: forall (f :: * -> *). Applicative f => (NonEmpty Text -> f (NonEmpty Text)) -> Text -> f Text Expected: Option -> Text -> Traversal' Text (NonEmpty Text) Actual: Option -> Text -> (NonEmpty Text -> f0 (NonEmpty Text)) -> Text -> f0 Text • In the expression: withMatcher $ \ matcher -> _capturesInternal matcher getAllSliceRanges slice In an equation for ‘_capturesOpt’: _capturesOpt = withMatcher $ \ matcher -> _capturesInternal matcher getAllSliceRanges slice | 1289 | _capturesOpt = withMatcher $ \matcher -> | ^^^^^^^^^^^^^^^^^^^^^^^^^... src/hs/Text/Regex/Pcre2/Internal.hs:1301:13: error: • Couldn't match type: (Text -> f1 Text) -> Text -> f1 Text with: forall (f :: * -> *). Applicative f => (Text -> f Text) -> Text -> f Text Expected: Option -> Text -> Traversal' Text Text Actual: Option -> Text -> (Text -> f1 Text) -> Text -> f1 Text • In the expression: withMatcher $ \ matcher -> _capturesInternal matcher get0thSliceRanges slice . _headNE In an equation for ‘_matchOpt’: _matchOpt = withMatcher $ \ matcher -> _capturesInternal matcher get0thSliceRanges slice . _headNE | 1301 | _matchOpt = withMatcher $ \matcher -> | ^^^^^^^^^^^^^^^^^^^^^^^^^... ```
sjshuck commented 3 years ago

Good catch, really appreciate the time you spent testing this with GHC 9. Only suggested style changes to make it consistent, and reduce granularity of helper functions.

sjshuck commented 3 years ago

I'm reminded of the ATraversal and similar types from lens. Maybe that would have been another solution? Not going to look into it now. Anyway, thanks @amesgen!

amesgen commented 3 years ago

Yes, that would work in principle, as ATraversal is monomorphic. But one would have to use cloneTraversal on use site when something Traversal-ish is required. Also, the standard use case for ATraversal is as an argument (in negative position) due to type inference, which is not the case here. This guide (uses Purescript, which has the same problem) is a good explanation: https://github.com/purescript-contrib/purescript-profunctor-lenses/blob/1f37af1adb4ad4c0611562481a62a9ddfa0930bd/docs/Impredicativity.md