Data.Generics.ConstrRep isn't general enough

[guest]

The type ConstrRep? in Data.Generics is used to represent constructors for types. The FloatConstr? constructor is the one used for all kinds of floating point primitives. But FloatConstr? has an argument of type Double. This limits the primitive floating point types to those that are accurately representable as a Double. In other places in Haskell where an accurate and generic floating point value is needed the type Rational is used. It should be used here too.

[simonpj]

Assigning to Jose.

[dreixel]

Attached patches to fix it. A user visible change is the type of function mkFloatConstr. It was previously 'DataType -> Double -> Constr', it's now '(Real a) => DataType -> a -> Constr'. This should cause no problems with existing code, though.

[igloo]

Hmm, it seems inconsistent to have these two:

mkIntConstr :: DataType -> Integer -> Constr
mkFloatConstr :: (Real a) => DataType -> a -> Constr

Shouldn't it either be

mkIntConstr :: DataType -> Integer -> Constr
mkFloatConstr :: DataType -> Rational -> Constr

or

mkIntConstr :: (Integral a) => DataType -> a -> Constr
mkFloatConstr :: (Real a) => DataType -> a -> Constr

?

The *Float* names are also confusing, but presumably the plan is to transition away from them?

[dreixel]

The problem with

mkFloatConstr :: DataType -> Rational -> Constr

is that it breaks existing code which uses 'mkFloatConstr'. I would then prefer to change 'mkIntConstr' to

mkIntConstr :: (Integral a) => DataType -> a -> Constr

instead.

I agree that the names are not ideal. We could also make them 'mkIntegralConstr' and 'mkRealConstr', deprecating the old ones, I guess.

[dreixel]

Attached a new patch which deprecates mkIntConstr and mkFloatConstr, and introduces mkIntegralConstr and mkRealConstr.

[igloo]

Applied, thanks!