
PS. Unique also looks like a case where Ints are used and (>= 0) is asserted. Can these cases be converted to Word as per earlier discussions? ________________________________ Van: p.k.f.holzenspies@utwente.nl
data UniqueClass
= UniqDesugarer
| UniqAbsCFlattener
| UniqSimplStg
| UniqNativeCodeGen
...
and a public (i.e. exported) function:
mkUnique :: UniqueClass -> Int -> Unique
? The benefit of this would be to have more (to my taste) self-documenting code and a greater chance that documentation is updated (the list of "unique supply characters" in the comments is currently outdated). 3) Is there a reason for having functions implementing class-methods to be exported? In the case of Unique, there is pprUnique and:
instance Outputable Unique where
ppr = pprUnique
Here pprUnique is exported and it is used in quite a few places where it's argument is unambiguously a Unique (so it's not to force the type) *and* "ppr" is used for all kinds of other types. I'm assuming this is an old choice making things marginally faster, but I would say cleaning up the API / namespace would now outweigh this margin.
I will also be adding Haddock-comments, so when this is done, a review would be most welcome (I'll also be doing some similar transformations to other long-since-untouched-code).
Regards,
Philip
________________________________
Van: Simon Peyton Jones
data Unique = MkUnique FastInt
1) As someone already commented: Is there a specific reason (other than history) that this isn't simply a newtype around an Int? If we're boxing anyway, we may as well use the default Int boxing and newtype-coerce to the specific purpose of Unique, no? 2) As a general question for GHC hacking style; what is the reason for hiding the constructors in the first place? I understand about abstraction and there are reasons for hiding, but there's a "public GHC API" and then there are all these modules that people can import at their own peril. Nothing is guaranteed about their consistency from version to version of GHC. I don't really see the point about hiding constructors (getting in the way of automatically deriving things) and then giving extra functions like (in the case of Unique):
getKeyFastInt (MkUnique x) = x
mkUniqueGrimily x = MkUnique (iUnbox x)
I would propose to just make Unique a newtype for an Int and making the constructor visible. Regards, Philip