cats
cats copied to clipboard
typelevel
Use SAM syntax for typeclass instances where possible
Definitions are shorter and don't generate anonymous classes in some cases.
Instances:
- [x]
Show - [x]
Eq - [x]
PartialOrder - [x]
Order - [x]
Semigroup,CommutativeSemigroup - [x] ~
Monoid,CommutativeMonoid~ - [x]
Band - [x]
Semilattice,BoundedSemilattice
Closes #3870
I'm all for using the sam syntax for things like val eqBool: Eq[Boolean] = { (a, b) => a == b } since (I believe) that is the same performance at runtime.
However, this is doing something different since we are not taking a function literal and returning a trait value, we are passing a literal to a function so it must have the indirection.
It may be inlined by the JIT, but my understanding is that Function1 being megamorphic that will not happen. So I'm concerned this is a performance degradation for a slight benefit (perhaps) in jar size and in lines of code.
I'm all for using the sam syntax for things like val eqBool: Eq[Boolean] = { (a, b) => a == b } since (I believe) that is the same performance at runtime.
Do you think we should do that instead? I would be in favour of that change.
yeah, I think that change would be fine.
Another idea, we could add a macro that we could write on 2.12/2.13 and 3.0 that would work like some of these constructor methods except we know that it will inline. So this could be useful for a more concise Monoid implementation for instance (since it isn't a SAM).
But honestly, that sounds like a lot of work for some pretty minor syntax wins.
I'm all for using the sam syntax for things like
val eqBool: Eq[Boolean] = { (a, b) => a == b }since (I believe) that is the same performance at runtime.
Yes, I believe it is equivalent in performance, but IIUC it still generates an anonymous class in Scala 2 due to the compiler bug described in https://github.com/typelevel/cats/pull/3871#issuecomment-980432480.
It may be inlined by the JIT, but my understanding is that Function1 being megamorphic that will not happen. So I'm concerned this is a performance degradation for a slight benefit (perhaps) in jar size and in lines of code.
Yup, my concern too, which I raised in https://github.com/typelevel/cats/pull/3871#issuecomment-979604249. There at least there were significant decreases in jar size.
