Add Data.Semigroup and Data.List.NonEmpty (re #10365)
ClosedPublic

Authored by hvr on Sep 27 2015, 5:11 AM.

Details

Summary

This implements phase 1 of the semigroup-as-monoid-superclass
proposal.

The modules were migrated from the semigroups-0.17 release mostly
as-is, except for dropping several {-# INLINE #-}s, removing CPP
usage, and instances for types & classes provided outside of
base (e.g. containers, deepseq, hashable, tagged,
bytestring, text)

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 4332.Sep 27 2015, 5:11 AM
hvr retitled this revision from to Add Data.Semigroup and Data.List.NonEmpty (re #10365).
hvr updated this object.
hvr edited the test plan for this revision. (Show Details)
hvr added a reviewer: ekmett.
hvr updated the Trac tickets for this revision.
bgamari edited edge metadata.Sep 27 2015, 5:41 AM

@hvr, out of curiosity what was the rationale for removing the INLINEs?

hvr added a comment.Sep 27 2015, 5:50 AM

@hvr, out of curiosity what was the rationale for removing the INLINEs?

They were applied to many trivial functions which the inliner would hopefully be inlining anyway. Or let me turn this question around, when do we need to explicitly mark something as INLINE (rather than e.g. INLINEABLE)?

libraries/base/Data/List/NonEmpty.hs
142

missed this one

163

missed this one

169

missed this one

hvr added a reviewer: quchen.Sep 27 2015, 6:47 AM
quchen edited edge metadata.Sep 27 2015, 6:54 AM
This comment was removed by quchen.
quchen added inline comments.Sep 27 2015, 7:05 AM
libraries/base/Data/Semigroup.hs
169

Which one is more desirable,

  • Partial function
  • Treating negative integers as 0

I'm not sure I like the error in a class definition.

ekmett edited edge metadata.Sep 27 2015, 6:54 PM

It is consistent with existing behavior

+Prelude> 4^(-7)
*** Exception: Negative exponent
austin added inline comments.Sep 29 2015, 8:50 AM
libraries/base/Data/List/NonEmpty.hs
14

This should be libraries@haskell.org

18–19

This is pretty suboptimal boiler-plate IMO. How about:

A @'NonEmpty'@ list is one which always has exactly one element, but is otherwise identical to the traditional list type in complexity and in terms of API. You will almost certainly want to import this module @qualified@.

21

Need to remember this should be changed to 4.9, although that won't happen until we branch I guess.

123

All top-level exports should have docs.

140

docs

144

more docs

171

docs, only other one missing in this module.

libraries/base/Data/Semigroup.hs
17

Switch Maintainer as above.

austin requested changes to this revision.Sep 29 2015, 8:50 AM
austin edited edge metadata.
This revision now requires changes to proceed.Sep 29 2015, 8:50 AM
hvr updated this revision to Diff 4348.Sep 29 2015, 10:34 AM
hvr edited edge metadata.
hvr marked 7 inline comments as done.
austin accepted this revision.Sep 30 2015, 2:37 PM
austin edited edge metadata.

Okay, LGTM. Might be worth speed testing the INLINE thing but I won't hold this up on that; I'd be sad if GHC couldn't inline easy things like this when possible :P

This revision is now accepted and ready to land.Sep 30 2015, 2:37 PM
ekmett accepted this revision.Oct 1 2015, 6:12 PM
ekmett edited edge metadata.

LGTM

libraries/base/Data/List/NonEmpty.hs
19–20

Hopefully, "at least one element"! =)

This revision was automatically updated to reflect the committed changes.
hvr added inline comments.Oct 2 2015, 1:27 AM
libraries/base/Data/List/NonEmpty.hs
21

There are several other references spread throughout base. I'll grep through them anyway if and when we bump to 4.9 :-)

libraries/base/Data/Semigroup.hs
169