Signed-off-by: Baldur Blöndal <firstname.lastname@example.org>
Hm, it looks like something messed up when the Diff was updated, since this only shows the changes in the last commit. Can you rebase your changes on top of master and arc diff --update D2268?
Also, base is built as a stage-2 dependency (i.e., with the latest GHC), so you can use TypeFamilyDependencies unconditionally.
Looks good! A couple of minor things:
First, make sure to add an entry to base's changelog describing this change (and Trac ticket number).
You don't really need to put Trac tickets in user-facing Haddocks, since they're mostly for developer reference. In fact, I'd opt to keep the documentation as it is, since Haddock will render type famiky dependencies anyway.
These are other potential type family dependencies I found.
From original paper, so I guess this should be added:
-- ./libraries/vector/Data/Vector/Generic/Base.hs type family Mutable (v :: * -> *) = (res :: * -> * -> *) | res -> v
-- ./libraries/base/GHC/Generics.hs class (kparam ~ 'KProxy) => SingKind (kparam :: KProxy k) where -- | Get a base type from a proxy for the promoted kind. For example, -- @DemoteRep ('KProxy :: KProxy Bool)@ will be the type @Bool@. type DemoteRep kparam = (res :: *) | res -> kparam
These are probably not super useful.
This could also be a closed type family, but since it's from old-testsuite it may not be worth changing.
-- ./libraries/vector/old-testsuite/Testsuite/Utils/Property.hs type family Ty a = res | res -> a
-- ./libraries/hoopl/src/Compiler/Hoopl/Collections.hs class IsSet set where type ElemOf set = res | res -> set class IsMap map where type KeyOf map = res | res -> map -- ./libraries/hoopl/src/Compiler/Hoopl/Unique.hs instance IsSet UniqueSet where type ElemOf UniqueSet = Unique instance IsMap UniqueMap where type KeyOf UniqueMap = Unique -- ./libraries/hoopl/src/Compiler/Hoopl/Label.hs instance IsSet LabelSet where type ElemOf LabelSet = Label instance IsMap LabelMap where type KeyOf LabelMap = Label
As for the other suggested type families:
- Mutable: I agree! However, vector is used as a submodule in GHC from its GitHub repo, so I'd make a PR there if you want to change it to be injective. (There's already this issue which pertains to the need for injective type families.)
- DemoteRep: Ostensibly, this looks like it could be injective. However, you might want to make a PR to the singletons repo first (where this code was taken) to make sure there isn't some strange corner case that violates injectivity. (In any case, DemoteRep is completely internal to GHC.Generics in base, so whether it's injective or not is inconsequential.)
- Ty: I'd avoid touching any testsuite code unless you have a very good reason to change it.
- ElemOf/KeyOf: Looks plausible, but again, make a PR request to the hoopl GitHub repo if you want to see this change.
I spoke with @hvr, and he agrees that 126.96.36.199 is what we should use for now.
@Iceland_jack, can you change the Haddocks for Not to something like this?
-- | Type-level "not". An injective type family since @188.8.131.52@. -- -- @since 184.108.40.206
(220.127.116.11 being the base version in which Not was introduced.)
By the way @Iceland_jack, I was curious about whether DemoteRep could be injective in the general case, so I tried marking it as injective in the upstream singletons repo. It turns out the answer is no:
[32 of 50] Compiling Data.Singletons.TypeLits.Internal ( src/Data/Singletons/TypeLits/Inte src/Data/Singletons/TypeLits/Internal.hs:1:1: error: Type family equations violate injectivity annotation: DemoteRep [a0] = [DemoteRep a0] -- Defined in ‘Data.Singletons.TypeLits.Internal’ DemoteRep Symbol = String -- Defined at src/Data/Singletons/TypeLits/Internal.hs:66:8
At least, as long as we keep the Symbol hack around.