Lazier Semigroup instance for Maybe

Mailing list discussion here: https://mail.haskell.org/pipermail/libraries/2018-May/028818.html

The existing Semigroup instance for Maybe is:

instance Semigroup a => Semigroup (Maybe a) where
    Nothing <> b       = b
    a       <> Nothing = a
    Just a  <> Just b  = Just (a <> b)

It has been proposed that it be replaced by:

instance Semigroup a => Semigroup (Maybe a) where
    Nothing <> b = b
    Just a  <> b = Just (maybe a (a<>) b)

This is lazier in the second argument, making it more consistent with the strictness of the Semigroup instances for And,Or,Ord,Either,Proxy and (). Equivalently, we could write:

instance Semigroup a => Semigroup (Maybe a) where
    Nothing <> b = b
    Just a  <> Nothing = Just a
    Just a  <> Just b  = Just (a <> b)

This makes it a little more clear that we aren't building a closure for partial function application, but it should have the same behavior.

Trac metadata

Trac field	Value
Version	8.2.2
Type	FeatureRequest
TypeOfFailure	OtherFailure
Priority	normal
Resolution	Unresolved
Component	Compiler
Test case
Differential revisions
BlockedBy
Related
Blocking
CC	ekmett
Operating system
Architecture

changed milestone to %8.6.1

changed weight to 5

added Tfeature request Trac import labels

The last version is not equivalent - it doesn't produce Just until the right argument has been evaluated. For example, with the former, I think foldMap (Just . (:[])) [1..] = Just [1..], but with the latter (and with the current one) it diverges.

changed milestone to %8.8.1

These won't be addressed for GHC 8.6.

changed milestone to %8.10.1

Bumping milestones of low-priority tickets.

added Pnormal label

removed milestone

marked this issue as related to #18265

added core libraries label

@andrewthad I just opened #18265 to document the current strictness since I've recently been surprised by it, but I tend to agree that the lazier instance would be better. Do you still think so too? Also, do you believe this is worth bringing up on the libraries mailing list again, and would you be interested in doing so?

Honestly, I lean toward just leaving it alone. Changing it would possibly cause a surprising performance degradation or space leak for someone, and I have no real examples of what this would actually improve.

CC @core-libraries

I agree with @andrewthad here. I would err on not doing anything, because this is a very widely used instance, and introducing more laziness would potentially cause leaks. Not that this is not an awful thing per se, but we must tread very carefully and make lots of noise.

I agree with the aforementioned lines of thinking that lead to "do nothing".

Seems like there's already a good bit of consensus that the instance should stay as it is. I admittedly share the concern that changing the strictness might cause a lot of subtle (perf) bugs.

Should this issue be closed then?

closed