Behavior of Semigroup/Monoid instances for Prod?

[chowells79]

I noticed that the Monoid and Semigroup instances for Prod duplicate the Alternative instance. I found that unexpected. I was expecting them to lift the element instances over the Applicative instance, a la (<>) = liftA2 (<>). That's the behavior of those instances in many other parsing libraries.

Changing the instances now would be awkward, of course. People might not see the change and think that just because their grammars still compile, the code still is correct. I'm not really asking for a change. Just wondering why this design was chosen, and suggesting that a change is something you might consider if you ever make large-scale changes that would break things in other ways.

[ollef]

Yes, that behaviour seems more useful than the current one, and it's better to use an established standard behaviour when we can. I can't remember if there's any good reason for the current design, but I don't think so. It was probably just the most obvious monoid I could think of at the time.

If we were able to deprecate uses of a type class instance it would be quite reasonable to change them after a deprecation period on the old instances, but I don't think that's possible with GHC right now.

Since that's not possible, perhaps we could remove them in a release and reintroduce the new instances after some time?

What do you think? Does anyone else have any input?

[sboosali]

why not document it and bump the major version? it seems cleaner than a gap in the presence of an instance (but I agree that it being misleading as a non-type-error change is problematic).

…

On Wed, Nov 28, 2018, 00:05 Olle Fredriksson ***@***.*** wrote: Yes, that behaviour seems more useful than the current one, and it's better to use an established standard behaviour when we can. I can't remember if there's any good reason for the current design, but I don't think so. It was probably just the most obvious monoid I could think of at the time. If we were able to deprecate uses of a type class instance it would be quite reasonable to change them after a deprecation period on the old instances, but I don't think that's possible with GHC right now. Since that's not possible, perhaps we could remove them in a release and reintroduce the new instances after some time? What do you think? Does anyone else have any input? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#39 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACNoMbKJEWixdJ14i1T2ZdEVBEMZ_qlvks5uzkPmgaJpZM4Y2sZc> .

[ollef]

Yeah, it's to avoid breaking people's code without compile errors.

I'm not sure how careful we need to be here. Are there even any users using the instances in question, and if they are, are they returning results that have Monoid or Semigroup instances?

[phadej]

• New instance has high chance of breaking users code already, not everything is Semigroup / Monoid.

• Hopefully all users have tests for their grammars, which will break if there are no compilation errors

• I'm strongly for direct major break.

• If @ollef has something else breaking in his mind, than it will help to break people properly, so they would read the changelog :)

[chowells79]

I would hazard a guess that few people are using those instances, but it's really hard to know. I've got no great insights to add regarding an upgrade path.