scala-collection-contrib
scala-collection-contrib copied to clipboard
scala
Improvements to the Scala 3 crossbuild
Exercise in -Xsource:3-cross.
~Not sure if~ it needs to stay on 3.3.3 LTS.
Scala 2 needs uninitialized. Scala 3 needs inline tap.
Scala 2 could support import chaining.given directly, though it doesn't matter for Scala 2 implicits. https://github.com/scala/bug/issues/12999
(I can be a reviewer of last resort here if needed, but I'm hoping one or more collections volunteers might step in)
If no one else will step in sooner, I will take a look tomorrow. At first sight it looks like a stuff I should be familiar with.
The change that is not syntactic is protected[this] to protected. I assume protected[this] is the most pernicious form of qualified access.
-Wnonunit-statement did its job, because cruft is generated and unused. The better solution is
elems.getOrElseUpdate(k, Set.empty[V]).addOne(v)
on the add side. Not sure about the subtract side, where you want the remove.
It indicates that updateWith is not great for internal operations.
Actually, it warns on the addOne because even though it's the this.type of the set, you don't know which set that is. It demands a stabilized value:
val vs = elems.getOrElseUpdate(k, Set.empty[V])
vs.addOne(v)
I don't see this as "unnecessary ceremony".
Use of tap is also a test of cross-compilation. Scala 3 ought already to intercept that API to make it inline, so this is a way to raise awareness.
Scala 3 ought already to intercept that API to make it inline, so this is a way to raise awareness.
Is that documented somewhere? https://www.scala-lang.org/api/current/scala/util/ChainingOps.html does not say a word about this. When I discussed in https://contributors.scala-lang.org/t/scala-3-specific-stdlib-improvements/6661/5, nobody mentioned that there.
Regarding -Wnonunit-statement, I guess it is your call. While some of the changes are not necessary, all changes look harmless to me.
On the warning, I think it's like saying, "Well, that unit test didn't uncover any issues, so just delete the test."
I forgot to retract the experiment with tap, though I think it's a worthwhile experiment.
For the updateWith, we're not really updating, we're getting and mutating. In the case where the mutated set or count is exhausted, we remove the key.
Reopening. To close this intentionaly I think it is at least worth a comment why (or a link to issue or PR supeseding this).
Looks like Som has lost interest; would a volunteer from @scala/collections like to finish it up in a replacement PR?
I can prepare a PR as described in:
The minimal PR porting to 2.13.14 while staying on 3.3.3 could just replace the library version in build.sbt (and perhaps update sbt version at the same time)
That should be just a few SBT lines.
o 2.13.14 while staying on 3.3.3 could just replace the library version in build.sbt (and perhaps update sbt version at the same time)
This is both already merged in master as PRs made by scala-steward (#245, #246)
Scala 3.3.3 is also already used since #234, also by scala-steward.
I can create a PR based on commits which avoid updateWith, as that makes sense to me and should make future maintenance easier.
One other change which might be beneficial is to use 2.3.14 stdlib even with Scala 3.3.3 (cf. https://users.scala-lang.org/t/what-scala-library-version-is-used-by-which-scala-3-version/9999/7)
@SethTisue @som-snytt any opinion on this?
One other change which might be beneficial is to use 2.3.14 stdlib even with Scala 3.3.3 (cf. https://users.scala-lang.org/t/what-scala-library-version-is-used-by-which-scala-3-version/9999/7)
@SethTisue @som-snytt any opinion on this?
Hmm... is there a specific reason to add the override? If not I'd just leave it alone and wait for whatever 3.3.4 gives us.
Hmm... is there a specific reason to add the override?
Not really. Just a way to sneakily force the 2.13.14 fixes on unsuspecting users.
@OndrejSpanel the 2.13.14 bump has been merged for inclusion in Scala 3.3.4 (and the Steward should give us that upgrade here once it's available, so I think we can just wait)