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

Bugfix in th-expand-syns-0.4.9.0 breaks th-reify-many #9

Closed
RyanGlScott opened this issue Aug 30, 2021 · 4 comments · Fixed by #10
Closed

Bugfix in th-expand-syns-0.4.9.0 breaks th-reify-many #9

RyanGlScott opened this issue Aug 30, 2021 · 4 comments · Fixed by #10

Comments

@RyanGlScott
Copy link
Collaborator

After uploading th-expand-syns-0.4.9.0, I noticed that th-orphans-0.13.11 now fails to compile:

[2 of 2] Compiling Language.Haskell.TH.Instances ( src/Language/Haskell/TH/Instances.hs, dist/build/Language/Haskell/TH/Instances.o, dist/build/Language/Haskell/TH/Instances.dyn_o )

src/Language/Haskell/TH/Instances.hs:478:2: error:
    Duplicate instance declarations:
      instance Lift a => Lift (Maybe a)
        -- Defined at src/Language/Haskell/TH/Instances.hs:478:2
      instance Lift a => Lift (Maybe a)
        -- Defined in ‘Language.Haskell.TH.Syntax’
    |
478 | $(reifyManyWithoutInstances ''Lift [''Info, ''Loc] (const True) >>=
    |  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^...

src/Language/Haskell/TH/Instances.hs:478:2: error:
    Duplicate instance declarations:
      instance Lift a => Lift (GHC.Real.Ratio a)
        -- Defined at src/Language/Haskell/TH/Instances.hs:478:2
      instance Integral a => Lift (GHC.Real.Ratio a)
        -- Defined in ‘Language.Haskell.TH.Syntax’
    |
478 | $(reifyManyWithoutInstances ''Lift [''Info, ''Loc] (const True) >>=
    |  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^...

Some investigation reveals that a minor bugfix in th-expand-syns-0.4.9.0 is affecting the behavior of th-reify-many, which has adverse consequences for libraries that depend on it (e.g., th-orphans). Here is a test program which illustrates the difference between th-expand-syns-0.4.8.0 and 0.4.9.0:

{-# LANGUAGE KindSignatures #-}
{-# LANGUAGE TemplateHaskell #-}
module Main (main) where

import Language.Haskell.TH (pprint, stringE)
import Language.Haskell.TH.ExpandSyns (expandSyns)

main :: IO ()
main = putStrLn
  $(do ty  <- [t| Maybe Int :: * |]
       ty' <- expandSyns ty
       stringE (pprint ty'))

With th-expand-syns-0.4.9.0, this will print out the following output, as expected:

(GHC.Maybe.Maybe GHC.Types.Int :: *)

But on th-expand-syns-0.4.8.0, this will instead print out the following output:

(GHC.Maybe.Maybe :: *) GHC.Types.Int

Notice that the kind signature has been pushed inwards.


What does any of this have to do with th-reify-many? It turns out that th-reify-many was silently relying on the buggy behavior of th-expand-syns-0.4.8.0 in order to work correctly in some cases. When you call reify on a class name (as th-expand-syns does here), it will reify some instance arguments with an outermost kind signature. For example, if we use the Lift example from earlier, we get:

λ> putStrLn $(reify ''Lift >>= stringE . pprint)
class Language.Haskell.TH.Syntax.Lift (t_0 :: GHC.Prim.TYPE r_1)
    where ...
...
instance GHC.Real.Integral a_61 => Language.Haskell.TH.Syntax.Lift (GHC.Real.Ratio a_61 :: *)
instance Language.Haskell.TH.Syntax.Lift a_63 => Language.Haskell.TH.Syntax.Lift (GHC.Maybe.Maybe a_63 :: *)
...

When checking if an instance has already been defined, th-reify-many will attempt to strip off these kind signatures (using the unSigT function) so as not to affect the results of (==). This happens in this line:

case tailMay $ map (fmap unSigT . headMay . unAppsT) $ unAppsT typ of

However, note that unSigT is called on the head of the application (i.e., the result of headMay), not on the overall application itself. In other words, this will remove the kind signature in (Maybe :: *) a, but it will not remove the kind signature in (Maybe a) :: *. Because of this, the call to unAppsT will not decompose the (Maybe a) :: * application, which causes th-reify-many to incorrectly believe that a Lift instance does not exist for Maybe.

One possible fix is to just call unSigT an additional time:

diff --git a/src/Language/Haskell/TH/ReifyMany/Internal.hs b/src/Language/Haskell/TH/ReifyMany/Internal.hs
index ae7b373..11ba090 100644
--- a/src/Language/Haskell/TH/ReifyMany/Internal.hs
+++ b/src/Language/Haskell/TH/ReifyMany/Internal.hs
@@ -100,7 +100,7 @@ lookupInstance xs n = headMay $ filter (`instanceMatches` n) xs
 -- the given 'TypeclassInstance'.
 instanceMatches :: TypeclassInstance -> Name -> Bool
 instanceMatches (TypeclassInstance _ typ _) n' =
-    case tailMay $ map (fmap unSigT . headMay . unAppsT) $ unAppsT typ of
+    case tailMay $ map (fmap unSigT . headMay . unAppsT . unSigT) $ unAppsT typ of
         Nothing -> False
         Just xs -> not $ null [() | Just (ConT n) <- xs, n == n']

This is sufficient to make th-orphans-0.13.11 compile again.

@phadej
Copy link

phadej commented Aug 31, 2021

I revised all the releases of th-reify-many with th-expand-syns <0.4.9.0, so the broken behavior doesn't spread.

@mgsloan
Copy link
Owner

mgsloan commented Sep 6, 2021

Fix released in 0.1.10. Thanks a bunch for noticing and fixing this @RyanGlScott !

mgsloan added a commit to mgsloan/stackage that referenced this issue Sep 6, 2021
Issue fixed in version 0.1.10 of th-reify-many (see mgsloan/th-reify-many#9)
@mgsloan
Copy link
Owner

mgsloan commented Sep 6, 2021

Also, I've added you as a collaborator on github and maintainer on hackage, which will expedite such fixes if they occur again in the future :)

@RyanGlScott
Copy link
Collaborator Author

Thanks, @mgsloan!

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 a pull request may close this issue.

3 participants