Clean-up `Data.Fixed`
ClosedPublic

Authored by hvr on Oct 28 2014, 3:29 AM.

Details

Summary

This gets rid of -fno-warn-unused-binds by turning the Ex types into
constructor-less data types (as they're used as phantom-types only)

Moreover, this modules uses AutoDeriveTypeable so we can drop all those
redundant deriving (Typeable) lines as well

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 1058.Oct 28 2014, 3:29 AM
hvr retitled this revision from to Clean-up `Data.Fixed`.
hvr updated this object.
hvr edited the test plan for this revision. (Show Details)
hvr added reviewers: ekmett, austin.

Not sure why you want to remove default (), but the rest looks good to me.

libraries/base/Data/Fixed.hs
46–47

Why remove this?

hvr added inline comments.Oct 28 2014, 11:27 AM
libraries/base/Data/Fixed.hs
46–47

Because this is most likely a leftover from when this was a NoImplicitPrelude module, where defaulting causes problems, or is there any value to keep default () around?

rwbarton added inline comments.Oct 28 2014, 1:49 PM
libraries/base/Data/Fixed.hs
46–47

I don't understand exactly why it was added in the first place, but I think in a library like base, it's better to be explicit about the types you want than to rely on defaulting, which might choose a type you didn't intend. And if you've gone to the trouble of writing defaulting-free code, why not have the compiler check it?

On the other hand, if validate's warning flags with -Werror will catch instances of defaulting anyways, then there's no need to do this on a per-module basis.

hvr updated this revision to Diff 1074.Oct 28 2014, 3:51 PM
hvr edited edge metadata.

readded default () to remain on the defensive side

austin accepted this revision.Oct 29 2014, 10:43 PM
austin edited edge metadata.

LGTM.

This revision is now accepted and ready to land.Oct 29 2014, 10:43 PM
ekmett accepted this revision.Oct 31 2014, 6:48 AM
ekmett edited edge metadata.

LGTM

This revision was automatically updated to reflect the committed changes.