From e1c55896bff03b3dcb7fa32e029a4dd5b7f3f30b Mon Sep 17 00:00:00 2001
From: odersky <odersky@gmail.com>
Date: Tue, 12 Dec 2023 10:57:18 +0100
Subject: [PATCH] Make more anonymous functions static

An anonymous function in a static object was previously mapped to a member
of that object. We now map it to a static member of the toplevel class instead.
This causes the backend to memoize the function, which fixes #19224. On the
other hand, we don't do that for anonymous functions nested in the object
constructor, since that can cause deadlocks (see run/deadlock.scala).

Scala 2's behavior is different: it does lift lambdas in constructors
to be static, too, which can cause deadlocks.

Fixes #19224
---
 .../tools/dotc/transform/Dependencies.scala   | 17 +++++++++-----
 tests/run/i19224.scala                        | 22 +++++++++++++++++++
 2 files changed, 34 insertions(+), 5 deletions(-)
 create mode 100644 tests/run/i19224.scala

diff --git a/compiler/src/dotty/tools/dotc/transform/Dependencies.scala b/compiler/src/dotty/tools/dotc/transform/Dependencies.scala
index fe429992fd46..57a0468422bb 100644
--- a/compiler/src/dotty/tools/dotc/transform/Dependencies.scala
+++ b/compiler/src/dotty/tools/dotc/transform/Dependencies.scala
@@ -1,4 +1,5 @@
-package dotty.tools.dotc
+package dotty.tools
+package dotc
 package transform
 
 import core.*
@@ -184,7 +185,6 @@ abstract class Dependencies(root: ast.tpd.Tree, @constructorOnly rootContext: Co
     def setLogicOwner(local: Symbol) =
       val encClass = local.owner.enclosingClass
       val preferEncClass =
-        (
           encClass.isStatic
             // non-static classes can capture owners, so should be avoided
           && (encClass.isProperlyContainedIn(local.topLevelClass)
@@ -192,15 +192,22 @@ abstract class Dependencies(root: ast.tpd.Tree, @constructorOnly rootContext: Co
               || encClass.is(ModuleClass, butNot = Package)
                   // needed to not cause deadlocks in classloader. see t5375.scala
               )
-        )
-        || (
+          && (!sym.isAnonymousFunction || sym.owner.ownersIterator.exists(_.isConstructor))
+            // For anonymous functions in static objects, we prefer them to be static because
+            // that means lambdas are memoized and can be serialized even if the enclosing object
+            // is not serializable. See run/lambda-serialization-gc.scala and run/i19224.scala.
+            // On the other hand, we don't want to lift anonymous functions from inside the
+            // object constructor to be static since that can cause deadlocks by its interaction
+            // with class initialization. See run/deadlock.scala, which works in Scala 3
+            // but deadlocks in Scala 2.
+        ||
           /* Scala.js: Never move any member beyond the boundary of a DynamicImportThunk.
            * DynamicImportThunk subclasses are boundaries between the eventual ES modules
            * that can be dynamically loaded. Moving members across that boundary changes
            * the dynamic and static dependencies between ES modules, which is forbidden.
            */
           ctx.settings.scalajs.value && encClass.isSubClass(jsdefn.DynamicImportThunkClass)
-        )
+          
       logicOwner(sym) = if preferEncClass then encClass else local.enclosingPackageClass
 
     tree match
diff --git a/tests/run/i19224.scala b/tests/run/i19224.scala
new file mode 100644
index 000000000000..3d076e35ee7e
--- /dev/null
+++ b/tests/run/i19224.scala
@@ -0,0 +1,22 @@
+// scalajs: --skip
+
+object Test extends App {
+  val field = 1
+  def x(): Int => String = (i: Int) => i.toString
+  def y(): () => String = () => field.toString
+
+  locally {
+    assert(x() == x()) // true on Scala 2, was false on Scala 3...
+    assert(y() == y()) // also true if `y` accesses object-local fields
+
+    def z(): Int => String = (i: Int) => i.toString
+    assert(z() != z()) // lambdas in constructor are not lifted to static, so no memoization (Scala 2 lifts them, though).
+  }
+
+  val t1 = new C
+  val t2 = new C
+
+  locally {
+    assert(t1.x() == t2.x()) // true on Scala 2, was false on Scala 3...
+  }
+}