Replace runtime.Rich* implicits on primitives by direct extension methods. #23872
Conversation
|
@sjrd you have beaten me to it 😄 |
|
The problem is that extension methods are not equivalent to implicit classes/definition extensions. Without solving https://contributors.scala-lang.org/t/relaxed-extension-methods-sip-54-are-not-relaxed-enough/6585/1 this change will break everywhere that has methods in the same name in scope. |
I don't see how that problem appears when defining extensions for monomorphic types. You wouldn't redefine another extension of |
| @deprecated("isWhole on Byte is always true", "2.12.15") | ||
| def isWhole: Boolean = true |
There was a problem hiding this comment.
I understand why this was done, but it feels weird to say that an extension method was deprecated since 2.12.15 😅
You are right. I thought the implementation used a typeclass approach + extension methods. My mistake. |
| * | ||
| * @param end The final bound of the range to make. | ||
| */ | ||
| def until(end: Byte): NumericRange.Exclusive[Byte] = NumericRange(self, end, 1) |
There was a problem hiding this comment.
Since it is part of the very little subset where everyone agrees on it :). Same goes for to.
| def until(end: Byte): NumericRange.Exclusive[Byte] = NumericRange(self, end, 1) | |
| infix def until(end: Byte): NumericRange.Exclusive[Byte] = NumericRange(self, end, 1) |
There was a problem hiding this comment.
This PR is strictly a refactoring. Adding infix anywhere should be done separately.
There was a problem hiding this comment.
If we view the PR this way, then okay.
| def max(that: Byte): Byte = java.lang.Math.max(self.toInt, that.toInt).toByte | ||
|
|
||
| /** Returns `this` if `this < that` or `that` otherwise. */ | ||
| def min(that: Byte): Byte = java.lang.Math.min(self.toInt, that.toInt).toByte |
There was a problem hiding this comment.
(my personal preferences here)
| def max(that: Byte): Byte = java.lang.Math.max(self.toInt, that.toInt).toByte | |
| /** Returns `this` if `this < that` or `that` otherwise. */ | |
| def min(that: Byte): Byte = java.lang.Math.min(self.toInt, that.toInt).toByte | |
| infix def max(that: Byte): Byte = java.lang.Math.max(self.toInt, that.toInt).toByte | |
| /** Returns `this` if `this < that` or `that` otherwise. */ | |
| infix def min(that: Byte): Byte = java.lang.Math.min(self.toInt, that.toInt).toByte |
sjrd
force-pushed
the
primitive-extensions
branch
2 times, most recently
from
7aade8f to
eb383fe
Compare
|
Will this change be available in 3.3.x LTS? |
Unfortunately, it is not possible to be backported to 3.3.x. |
|
No. |
sjrd
force-pushed
the
primitive-extensions
branch
2 times, most recently
from
51be203 to
bb19ec5
Compare
|
@WojciechMazur Would you mind running an open community build in this PR? It can affect source compatibility pretty strongly, so I'd like to gather cases where we can improve it. |
|
The first run of OpenCB was ineffective - we've finally started to require to use new minimal JDK version, so ~300 projects failed with |
We have been compiling the stdlib with Java 17 for two months now, it's just about compiling those projects with Java 17, not 8 (shouldn't be a problem unless weird project specific issues) |
In OpenCommmunityBuild we've been compiling projects using JDK 17 since 3.7 so that's not an issue. JDK 8 is currently only used for the 3.3 LTS OpenCB builds. For nightlies we use either 17 or 21 - version is picked based on GitHub workflows defined for project. We'd be adding JDK 25 soon as well. What has changed recently is we've finally started to enforce the new minimal version in #24146. Until that point The fix is on the way, I'm testing it locally, should be able to run OpenCB with it later today. |
…hods. It has always been weird that the public API of primitive types, enriched by `Predef` depends on the `Rich*` classes, which are in the not-really-public `runtime` package. Moreover, the design of those classes was overabstracted, with consequences on performance (unnecessary boxing) and quality of the API (including methods that do not make sense on some types). To mitigate that, the individual `Rich*` classes redefined some (but not all) of the methods, defeating the abstraction. We solve both issues with a simple solution: define all those methods as simple `extension` methods. We do this directly in the companion objects of the primitive types.
sjrd
force-pushed
the
primitive-extensions
branch
from
bb19ec5 to
e1ec5ec
Compare
|
When compared with latest nightly 3.8.0-RC1-bin-20251010-7b7f2f1-NIGHTLY there are 7 projects affected by this change (there might be more but these were already failing when using latest nightly)
|
|
Thanks. There are definitely a few things to improve there. |
5 participants
It has always been weird that the public API of primitive types, enriched by
Predefdepends on theRich*classes, which are in the not-really-publicruntimepackage.Moreover, the design of those classes was overabstracted, with consequences on performance (unnecessary boxing) and quality of the API (including methods that do not make sense on some types). To mitigate that, the individual
Rich*classes redefined some (but not all) of the methods, defeating the abstraction.We solve both issues with a simple solution: define all those methods as simple
extensionmethods. We do this directly in the companion objects of the primitive types.This would be a required step before we do anything about #23824.