Add definitions for stimes

[konsumlamm]

Resolves part of #307.

I added INLINE pragmas, for consistency with the other definitions, although they're probably not needed.

Also removed two unused {-# LANGUAGE LambdaCase #-}.

EDIT: I originally also added definitions for mconcat and sconcat, but removed them again (see discussion below).

[sjakobi]

Thanks!

The stimes definitions look good, but I'm less sure with sconcat and mconcat.

For these I think it would be best to have some tests to check the Semigroup and Monoid laws. We can probably use checkers or quickcheck-classes for this. (The transitive semigroupoids dependency may need to be configured with -f-unordered-containers.)

The performance implications also are unclear to me. I think it would be best to resolve #139 before using unions in mconcat and sconcat.

Do you want to work on this? If you don't (which would be perfectly fine!), I suggest you remove the changes to mconcat and sconcat for now.

[treeowl]

@sjakobi it all looks fine to me. I agree tests would be good (for stimes as well).

[sjakobi]

@treeowl What do you think about #139? Please do chime in on the discussion there!

[konsumlamm]

For these I think it would be best to have some tests to check the Semigroup and Monoid laws.

Are you referring to the mconcat = foldr (<>) mempty law? This would be broken with my implementation, although other packages (e.g. containers) don't respect this law either, so idk how important this is for you.

The performance implications also are unclear to me. I think it would be best to resolve #139 before using unions in mconcat and sconcat.

Do you want to work on this? If you don't (which would be perfectly fine!), I suggest you remove the changes to mconcat and sconcat for now.

Good point, I'll remove the mconcat/sconcat definitions (#139 seems to be non-trivial to solve).

[treeowl]

I'm just saying that we have to be careful not to change the current semantics (whatever they are) unintentionally.

[sjakobi]

For these I think it would be best to have some tests to check the Semigroup and Monoid laws.

Are you referring to the mconcat = foldr (<>) mempty law? This would be broken with my implementation, although other packages (e.g. containers) don't respect this law either, so idk how important this is for you.

I was referring to that law and the other ones documented in base. How does containers break the law BTW? Please make a bug report!

[sjakobi]

So the semantic change resulting from the explicit stimes definitions is that we now have

stimes 0 x = mempty

instead of before

stimes 0 x = undefined

I'd be comfortable with including this in a minor release. containers did the same with Seq in v0.6.1.1 (haskell/containers@c9d1e69). What do you think, @treeowl?

[treeowl]

Yeah, a semantic improvement is totally fine. I'm talking about the left vs. right bias in sconcat/mconcat. If we change it, we need to document, make a major version, etc.

[konsumlamm]

How does containers break the law BTW? Please make a bug report!

I was wrong, it doesn't.

If we just define mconcat = unions (with the current definition of unions), that should have the same behaviour (since union is associative). However, if we decide to define unions = foldl' (flip union) empty (which is what was originally suggested in #139), that wouldn't be the case anymore (although that would also be a breaking change for unions).

[treeowl]

@konsumlamm I imagine that we could define a unionOtherWay that's structured like union but prefers the other map. It would be interesting to see if a flipped version of that did a better job than the current unions.

[sjakobi]

Thank you, @konsumlamm! :)