From 4e6de64e46bd3479fccd9dfc704403801c6b7b96 Mon Sep 17 00:00:00 2001
From: odersky <odersky@gmail.com>
Date: Tue, 12 Dec 2023 09:56:55 +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 #19924
---
 .../tools/dotc/transform/Dependencies.scala     | 17 ++++++++++++-----
 tests/run/i19224.scala                          | 11 +++++++++++
 2 files changed, 23 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..3d34a278f7fc
--- /dev/null
+++ b/tests/run/i19224.scala
@@ -0,0 +1,11 @@
+
+object Test extends App:
+  val field = 1
+  def x(): Int => String = (i: Int) => i.toString
+  def y(): () => String = () => field.toString
+  locally:
+    assert(x() == x()) // was true on Scala 2, false on Scala 3...
+    assert(y() == y()) // also true if `y` accesses object-local fields
+
+
+//@main def Test = App()