Upgrade to latest scala versions #12401
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
mkurz
commented
Feb 26, 2024
mkurz
commented
- https://contributors.scala-lang.org/t/3-3-2-release-thread/6433/
- https://contributors.scala-lang.org/t/scala-2-13-13-release-planning/6315/
- https://contributors.scala-lang.org/t/scala-2-12-19-release-planning/6216/
Closed
mkurz
force-pushed
the
scala_upgrades
branch
from
fc7b995
to
2e80041
Compare
|
@SethTisue Seems there is a problem... Not sure if MiMa needs to be adjusted? If you run on this PR, you end up with: It's complaining about: |
|
Does it show up with |
|
The MiMa error reproduces for me. I compared |
mkurz
force-pushed
the
scala_upgrades
branch
from
2e80041
to
c5add76
Compare
|
useful command for exploring differences between the old and new version: first diff --git play/api/inject/SimpleModule.class.asm play/api/inject/SimpleModule.class.asm
index 389eb7c..edfd7c6 100644
--- play/api/inject/SimpleModule.class.asm
+++ play/api/inject/SimpleModule.class.asm
@@ -21,7 +21,10 @@
public <init>(Lscala/collection/immutable/Seq;)V
// access flags 0x11
- // signature (Lplay/api/Environment;Lplay/api/Configuration;)Lscala/collection/Seq<Lplay/api/inject/Binding<*>;>;
- // declaration: scala.collection.Seq<play.api.inject.Binding<?>> bindings(play.api.Environment, play.api.Configuration)
- public final bindings(Lplay/api/Environment;Lplay/api/Configuration;)Lscala/collection/Seq;
+ // signature (Lplay/api/Environment;Lplay/api/Configuration;)Lscala/collection/immutable/Seq<Lplay/api/inject/Binding<*>;>;
+ // declaration: scala.collection.immutable.Seq<play.api.inject.Binding<?>> bindings(play.api.Environment, play.api.Configuration)
+ public final bindings(Lplay/api/Environment;Lplay/api/Configuration;)Lscala/collection/immutable/Seq;
+
+ // access flags 0x1041
+ public synthetic bridge bindings(Lplay/api/Environment;Lplay/api/Configuration;)Lscala/collection/Seq;
}diff --git play/api/inject/SimpleModule.class.scalap play/api/inject/SimpleModule.class.scalap
index aa962b0..6aa4e30 100644
--- play/api/inject/SimpleModule.class.scalap
+++ play/api/inject/SimpleModule.class.scalap
@@ -2,5 +2,5 @@
class SimpleModule extends play.api.inject.Module {
def this(bindingsFunc: scala.Function2[play.api.Environment, play.api.Configuration, scala.Seq[play.api.inject.Binding[_]]]) = { /* compiled code */ }
def this(bindings: play.api.inject.Binding[_]*) = { /* compiled code */ }
- final override def bindings(environment: play.api.Environment, configuration: play.api.Configuration): scala.collection.Seq[play.api.inject.Binding[_]] = { /* compiled code */ }
+ final override def bindings(environment: play.api.Environment, configuration: play.api.Configuration): scala.Seq[play.api.inject.Binding[_]] = { /* compiled code */ }
} |
|
There's a |
|
Another observation: sbt "++2.13.x ; Play / mimaReportBinaryIssues"Works with Scala 3.3.2: sbt "++3.x ; Play / mimaReportBinaryIssues" |
|
Explicitly defining the return type of the overridden diff --git a/core/play/src/main/scala/play/api/controllers/Assets.scala b/core/play/src/main/scala/play/api/controllers/Assets.scala
index 48ad96ba9b..67462d3547 100644
--- a/core/play/src/main/scala/play/api/controllers/Assets.scala
+++ b/core/play/src/main/scala/play/api/controllers/Assets.scala
@@ -32,6 +32,7 @@ import org.apache.pekko.stream.scaladsl.StreamConverters
import play.api._
import play.api.http._
import play.api.inject.ApplicationLifecycle
+import play.api.inject.Binding
import play.api.inject.Module
import play.api.libs._
import play.api.mvc._
@@ -42,7 +43,7 @@ import play.utils.Resources
import play.utils.UriEncoding
class AssetsModule extends Module {
- override def bindings(environment: Environment, configuration: Configuration) = Seq(
+ override def bindings(environment: Environment, configuration: Configuration): scala.collection.Seq[Binding[_]] = Seq(
bind[Assets].toSelf,
bind[AssetsMetadata].toProvider[AssetsMetadataProvider],
bind[AssetsFinder].toProvider[AssetsFinderProvider],
diff --git a/core/play/src/main/scala/play/api/i18n/I18nModule.scala b/core/play/src/main/scala/play/api/i18n/I18nModule.scala
index 61ccec8f6d..8daec41eee 100644
--- a/core/play/src/main/scala/play/api/i18n/I18nModule.scala
+++ b/core/play/src/main/scala/play/api/i18n/I18nModule.scala
@@ -5,12 +5,13 @@
package play.api.i18n
import play.api.http.HttpConfiguration
+import play.api.inject.Binding
import play.api.inject.Module
import play.api.Configuration
import play.api.Environment
class I18nModule extends Module {
- def bindings(environment: Environment, configuration: Configuration) = {
+ def bindings(environment: Environment, configuration: Configuration): scala.collection.Seq[Binding[_]] = {
Seq(
bind[Langs].toProvider[DefaultLangsProvider],
bind[MessagesApi].toProvider[DefaultMessagesApiProvider],
diff --git a/core/play/src/main/scala/play/api/inject/Module.scala b/core/play/src/main/scala/play/api/inject/Module.scala
index 7011c78542..b8c1f06ca7 100644
--- a/core/play/src/main/scala/play/api/inject/Module.scala
+++ b/core/play/src/main/scala/play/api/inject/Module.scala
@@ -90,7 +90,7 @@ abstract class Module {
class SimpleModule(bindingsFunc: (Environment, Configuration) => Seq[Binding[_]]) extends Module {
def this(bindings: Binding[_]*) = this((_, _) => bindings)
- final override def bindings(environment: Environment, configuration: Configuration) =
+ final override def bindings(environment: Environment, configuration: Configuration): scala.collection.Seq[Binding[_]] =
bindingsFunc(environment, configuration)
} |
|
Pushed the fix, should work now. @SethTisue @lrytz @sjrd Is that a 2.13.13 release blocker? |
Otherwise Scala 2.13.13 will create wrong bytecode
mkurz
force-pushed
the
scala_upgrades
branch
from
4136ca8
to
cf46644
Compare
|
Given the fix, it does look like a type inference change. Type inference is not guaranteed to never change, so IMO this is not a blocker. |
|
Looking at Under the revised option names, EDIT: oh maybe you're going in the other direction, that is, your previous |
|
as suggested by Som, I've strengthened the wording about |
mkurz
force-pushed
the
scala_upgrades
branch
from
6863ee3
to
fe73187
Compare
mkurz
force-pushed
the
scala_upgrades
branch
from
fe73187
to
a1ca2b3
Compare
|
Tried Click to expand logWe don't want to introduce that much incompatibilities right now for existing 2.13.x users, so I just keep my original "fix" in place instead. |
|
@Mergifyio backport 2.9.x |
✅ Backports have been created
|
@mkurz with ➜ scala13 s -Xsource:3
Welcome to Scala 2.13.13 -Xsource:3.0.0 (OpenJDK 64-Bit Server VM, Java 21.0.1).
Type in expressions for evaluation. Or try :help.
scala> trait T { def f: Object }
scala> class C extends T { def f = "" }
^
error: under -Xsource:3-cross, the inferred type changes to Object instead of String [quickfixable]
Scala 3 migration messages are errors under -Xsource:3. Use -Wconf / @nowarn to filter them or add -Xmigration to demote them to warnings.
Applicable -Wconf / @nowarn filters for this fatal warning: msg=<part of the message>, cat=scala3-migration, site=C.f
➜ scala13 s -Xsource:3-cross
Welcome to Scala 2.13.13 -Xsource:3-cross (OpenJDK 64-Bit Server VM, Java 21.0.1).
Type in expressions for evaluation. Or try :help.
scala> trait T { def f: Object }
scala> class C extends T { def f = "" }
scala> (new C).f
val res0: Object = "" // static type Object |
|
@lrytz I understand that. What I meant in #12401 (comment) (if you look at the logs) is that |
|
Ah, I didn't look closely enough, thanks. So this is a scenario we didn't consider (cc @SethTisue, @som-snytt). Under case class A(x: Int)
object A extends scala.runtime.AbstractFunction1[Int, A] {
final override def toString = "A"
}But applying that change to the source code changes the bytecode produced by Scala 3. How can a project move to |
|
Lukas and I were just discussing this. Lukas will open a scala/bug ticket today with details and we'll consider how to handle this. It seems we'll need to roll 2.13.14 before long in order to address this, since we don't see any workaround for existing users of Anyway, let's discuss on the scala/bug ticket. (UPDATE: ticket is scala/bug#12961) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.