Add public rnf/hash operations to TypeRep/TyCon
ClosedPublic

Authored by hvr on Mar 4 2015, 2:57 AM.

Details

Summary

TyCon and TypeRep are supposed to be abstract, by providing these
additional few public operations the need to import
Data.Typeable.Internal is reduced, and future changes to the internal
structure of TypeRep/TyCon shouldn't require changes in packages such as
deepseq or hashable anymore (hopefully).

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
hvr updated this revision to Diff 2349.Mar 4 2015, 2:57 AM
hvr retitled this revision from to Add `rnfTy{Con,peRep}` & `hashTypeRep` to `Data.Typeable`.
hvr updated this object.
hvr edited the test plan for this revision. (Show Details)
hvr added reviewers: ekmett, austin, simonpj.
simonpj accepted this revision.Mar 4 2015, 4:11 AM
simonpj edited edge metadata.

Yes!

This revision is now accepted and ready to land.Mar 4 2015, 4:11 AM
hvr added a comment.Mar 4 2015, 6:04 AM

Fwiw, I'm tempted to also add hashTyCon :: TyCon -> Int

I'm just wondering if we shouldn't rather just expose :: Ty... -> Fingerprint accessors... or would that be already too concrete?

In D699#19505, @hvr wrote:

Fwiw, I'm tempted to also add hashTyCon :: TyCon -> Int

I'm just wondering if we shouldn't rather just expose :: Ty... -> Fingerprint accessors... or would that be already too concrete?

I don't have a strong opinion. A fingerprint is just a long hash-code! If we do, we should so so for TypeRep too., Maybe ask the CLC?

ekmett accepted this revision.Mar 4 2015, 11:39 AM
ekmett edited edge metadata.

I have no problem with exporting the fingerprint extraction method. Any instability around something that obscure can be dealt with by users dedicated enough to understand it in the first place.

We may have to eventually change out the hash function used for Fingerprint to reduce the possibility of birthday attacks subverting the type system due to https://ghc.haskell.org/trac/ghc/ticket/7634 but that wouldn't affect merely getting your hands on the fingerprint.

hvr planned changes to this revision.Mar 5 2015, 2:23 AM

I think I'll go for the more flexible/generic -> Fingerprint field accessors then...

hvr updated this revision to Diff 2356.Mar 5 2015, 2:47 AM
hvr edited edge metadata.

expose field-accessors to Fingerprint field instead of providing Int-hash functions

naming is a bit awkward but follows from the existing tyConHash field-name

This revision is now accepted and ready to land.Mar 5 2015, 2:47 AM
hvr retitled this revision from Add `rnfTy{Con,peRep}` & `hashTypeRep` to `Data.Typeable` to Add public rnf/hash operations to TypeRep/TyCon .Mar 5 2015, 2:49 AM
hvr updated this object.
hvr added a reviewer: tibbe.
austin accepted this revision.Mar 5 2015, 8:48 AM
austin edited edge metadata.

LGTM.

This revision was automatically updated to reflect the committed changes.
tibbe edited edge metadata.Mar 6 2015, 4:47 PM

LGTM