#14869 closed bug (fixed)

Documentation for isLiftedTypeKind is incorrect

Reported by: RyanGlScott Owned by:
Priority: normal Milestone: 8.6.1
Component: Compiler (Type checker) Version: 8.2.2
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Documentation bug Test Case: th/T14869
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D4474
Wiki Page:

Description (last modified by RyanGlScott)

I noticed recently that Template Haskell reifies Constraint as Type:

$ ~/Software/ghc2/inplace/bin/ghc-stage2 --interactive
GHCi, version 8.5.20180221: http://www.haskell.org/ghc/  :? for help
Loaded GHCi configuration from /home/rgscott/.ghci
λ> :set -XTypeFamilies -XTemplateHaskell
λ> :m + Data.Kind Language.Haskell.TH
λ> type family Foo :: Constraint
λ> putStrLn $(reify ''Foo >>= stringE . pprint)
type family Ghci1.Foo :: *

The root of the issue can be traced back to the isLiftedTypeKind function, which TcSplice uses to distinguish Type from Constraint. At least, that's what its documentation claims:

 -- | This version considers Constraint to be distinct from *. Returns True
 -- if the argument is equivalent to Type and False otherwise.
 isLiftedTypeKind :: Kind -> Bool
 isLiftedTypeKind = is_TYPE is_lifted
   where
     is_lifted (TyConApp lifted_rep []) = lifted_rep `hasKey` liftedRepDataConKey
     is_lifted _                        = False

However, in practice this claim about treating Constraint and Type as distinct is false:

$ ~/Software/ghc2/inplace/bin/ghc-stage2 --interactive -package ghc
GHCi, version 8.5.20180221: http://www.haskell.org/ghc/  :? for help
Loaded GHCi configuration from /home/rgscott/.ghci
λ> :m + TyCoRep TysWiredIn
λ> isLiftedTypeKind liftedTypeKind
True
λ> isLiftedTypeKind constraintKind
True

Either we should change the implementation of isLiftedTypeKind to match the documentation's claim, or change the documentation.

Change History (6)

comment:1 Changed 22 months ago by RyanGlScott

Description: modified (diff)

comment:2 Changed 22 months ago by RyanGlScott

Amusingly, the Constraint counterpart, isLiftedTypeKind, actually appears to do the right thing:

isConstraintKind :: Kind -> Bool
isConstraintKindCon :: TyCon -> Bool

isConstraintKindCon   tc = tyConUnique tc == constraintKindTyConKey

isConstraintKind (TyConApp tc _) = isConstraintKindCon tc
isConstraintKind _               = False

This actually checks the unique of the tycon, and per Note [Kind Constraint and kind *], the uniques for Type and Constraint are different. Thus:

$ inplace/bin/ghc-stage2 --interactive -package ghc
GHCi, version 8.5.20180221: http://www.haskell.org/ghc/  :? for help
Loaded GHCi configuration from /home/rgscott/.ghci
λ> :m + Kind TysWiredIn
λ> isConstraintKind liftedTypeKind
False
λ> isConstraintKind constraintKind
True

Thus, to fix the aforementioned Template Haskell bug, all we'd have to do is make this change:

  • compiler/typecheck/TcSplice.hs

    diff --git a/compiler/typecheck/TcSplice.hs b/compiler/typecheck/TcSplice.hs
    index 45e18e6..b052e8e 100644
    a b reifyFamilyInstance is_poly_tvs inst@(FamInst { fi_flavor = flavor 
    17071707------------------------------
    17081708reifyType :: TyCoRep.Type -> TcM TH.Type
    17091709-- Monadic only because of failure
    1710 reifyType ty                | isLiftedTypeKind ty = return TH.StarT
    1711                             | isConstraintKind ty = return TH.ConstraintT
     1710reifyType ty                | isConstraintKind ty = return TH.ConstraintT
     1711                            | isLiftedTypeKind ty = return TH.StarT
    17121712reifyType ty@(ForAllTy {})  = reify_for_all ty
    17131713reifyType (LitTy t)         = do { r <- reifyTyLit t; return (TH.LitT r) }
    17141714reifyType (TyVarTy tv)      = return (TH.VarT (reifyName tv))

I propose that we:

  1. Change the documentation for isLiftedTypeKind to reflect the fact that it does not distinguish between Constraint and Type.
  2. Make the above change to TcSplice to fix the bug.

Does that sound reasonable?

comment:3 Changed 22 months ago by simonpj

I propose that we:

Yes that sounds reasonable. Thanks

comment:4 Changed 22 months ago by RyanGlScott

Differential Rev(s): Phab:D4474
Status: newpatch

comment:5 Changed 21 months ago by Ryan Scott <ryan.gl.scott@…>

In 49ac3f0f/ghc:

Fix #14869 by being more mindful of Type vs. Constraint

Summary:
Before, we were using `isLiftedTypeKind` in `reifyType`
before checking if a type was `Constraint`. But as it turns out,
`isLiftedTypeKind` treats `Constraint` the same as `Type`, so every
occurrence of `Constraint` would be reified as `Type`! To make things
worse, the documentation for `isLiftedTypeKind` stated that it
treats `Constraint` //differently// from `Type`, which simply isn't
true.

This revises the documentation for `isLiftedTypeKind` to reflect
reality, and defers the `isLiftedTypeKind` check in `reifyType` so
that it does not accidentally swallow `Constraint`.

Test Plan: make test TEST=T14869

Reviewers: goldfire, bgamari

Reviewed By: goldfire

Subscribers: rwbarton, thomie, carter

GHC Trac Issues: #14869

Differential Revision: https://phabricator.haskell.org/D4474

comment:6 Changed 21 months ago by RyanGlScott

Milestone: 8.6.1
Resolution: fixed
Status: patchclosed
Test Case: th/T14869
Note: See TracTickets for help on using tickets.