Adding overloads to value class breaks binary compatibility

Compiler
1

I just came across this on Cats. @mpilquist pointed out that adding overloads to value classes breaks binary compatibility. I googled but couldn’t find a related issue. Where can I find some discussion on this? (I am asking because it seems to me that it could be avoided.)
Thanks!

4 Likes
2

Should this be fixed in Scala 2.13.0? Why are the overloaded methods generated with an extra digit? Wouldn’t Java’s regular method overloading be enough?

3

That and overloading with default arguments would be very nice to have fixed in the next version of Scala

4

2.13.0-RC1 will be released in a couple of weeks, it’s way too late for changing this now.

5

Would there be a reason for it not to be appropriate for 2.14, then?

6

No. Would you be interested in implementing this change?

7

Hi @dwijnand,

I tried it but got test failures in seemingly unrelated tests that involve the standard library:

scala.tools.nsc.backend.jvm.OptimizedBytecodeTest.optimiseEnablesNewOpt
scala.tools.nsc.backend.jvm.opt.InlinerTest.optimizeSpecializedClosures
scala.tools.nsc.backend.jvm.opt.InlinerTest.cleanRangeForeach

The error messages are similar to the ones below.

Error messages 
[error] Test scala.tools.nsc.backend.jvm.OptimizedBytecodeTest.optimiseEnablesNewOpt failed: java.lang.AssertionError: assertion failed:
[error]   no extension method found for:
[error]
[error]   method to:(end: Int)scala.collection.immutable.Range.Inclusive
[error]
[error]  Candidates:
[error]
[error]
[error]
[error]  Candidates (signatures normalized):
[error]
[error]
[error]
[error]  Eligible Names: to$extension,to$extension"
[error]      while compiling: unitTestSource.scala
[error]         during phase: globalPhase=erasure, enteringPhase=refchecks
[error]      library version: version 2.13.0-20190314-071759-152fdf4
[error]     compiler version: version 2.13.0-20190314-071759-152fdf4
[error]   reconstructed args: -optimise -opt-inline-from:** -opt:unreachable-code -opt:simplify-jumps -opt:compact-locals -opt:copy-propagation -opt:redundant-casts -opt:box-unbox -opt:nullness-tracking -opt:closure-invocations -opt:allow-skip-core-module-init -opt:assume-modules-non-null -opt:allow-skip-class-loading -opt:inline -usejavacp -deprecation
[error]
[error]   last tree to typer: Select(Apply(method to), foreach$mVc$sp)
[error]        tree position: line 1 of unitTestSource.scala
[error]             tree tpe: (f: Int => Unit)Unit
[error]               symbol: (final override) method foreach$mVc$sp in class Range
[error]    symbol definition: final override def foreach$mVc$sp(f: Int => Unit): Unit (a MethodSymbol)
[error]       symbol package: scala.collection.immutable
[error]        symbol owners: method foreach$mVc$sp -> class Range
[error]            call site: method t in class C in package <empty>
[error]
[error] == Source file context for tree position ==
[error]
[error]      1 class C { def t = (1 to 10) foreach println }
[error]      2 , took 0.49 sec

[error] Test scala.tools.nsc.backend.jvm.opt.InlinerTest.cleanRangeForeach failed: java.lang.AssertionError: assertion failed:
[error]   no extension method found for:
[error]
[error]   method to:(end: Int)scala.collection.immutable.Range.Inclusive
[error]
[error]  Candidates:
[error]
[error]
[error]
[error]  Candidates (signatures normalized):
[error]
[error]
[error]
[error]  Eligible Names: to$extension,to$extension"
[error]      while compiling: unitTestSource.scala
[error]         during phase: globalPhase=erasure, enteringPhase=refchecks
[error]      library version: version 2.13.0-20190314-071759-152fdf4
[error]     compiler version: version 2.13.0-20190314-071759-152fdf4
[error]   reconstructed args: -opt-inline-from:** -opt:unreachable-code -opt:simplify-jumps -opt:compact-locals -opt:copy-propagation -opt:redundant-casts -opt:box-unbox -opt:nullness-tracking -opt:closure-invocations -opt:allow-skip-core-module-init -opt:assume-modules-non-null -opt:allow-skip-class-loading -opt:inline -usejavacp -opt-warnings:at-inline-failed
[error]
[error]   last tree to typer: Select(Apply(method to), foreach$mVc$sp)
[error]        tree position: line 4 of unitTestSource.scala
[error]             tree tpe: (f: Int => Unit)Unit
[error]               symbol: (final override) method foreach$mVc$sp in class Range
[error]    symbol definition: final override def foreach$mVc$sp(f: Int => Unit): Unit (a MethodSymbol)
[error]       symbol package: scala.collection.immutable
[error]        symbol owners: method foreach$mVc$sp -> class Range
[error]            call site: method t in class C in package <empty>
[error]
[error] == Source file context for tree position ==
[error]
[error]      1 class C {
[error]      2   def t = {
[error]      3     var x = 0
[error]      4     for (i <- 1 to 10) x += i
[error]      5     x
[error]      6   }
[error]      7 }, took 0.084 sec

I wonder it they need some sort of re-bootstrapping, so the standard library gets compiled with the new identifiers and then these tests can target these new identifiers. May you give some tips on how to do it?

8

It’s hard to say without knowing what you changed.

I’d suggest you open a pull request in scala/scala so we can see your code changes alongside the CI results.

You can use GitHub’s new “draft pull request” feature to indicate that you know the PR isn’t merge-ready.

2 Likes
9

Here’s my PR - https://github.com/scala/scala/pull/7892

I saw the same test failure as you did, but I think it shows the limitation of the current build setup than the change itself. If I restarr (publishLocal, and then sbt -Dstarr.version=2.13.0-pre-SNAPSHOT) I can get those tests to pass.

1 Like
10

For the record: this was fixed in 2.13.x branch, by the combination of https://github.com/scala/scala/pull/7896 and https://github.com/scala/scala/pull/7922

1 Like