Skip to content

Commit

Permalink
Make more anonymous functions static
Browse files Browse the repository at this point in the history
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
  • Loading branch information
odersky committed Dec 13, 2023
1 parent 55c2002 commit e1c5589
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 5 deletions.
17 changes: 12 additions & 5 deletions compiler/src/dotty/tools/dotc/transform/Dependencies.scala
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
package dotty.tools.dotc
package dotty.tools
package dotc
package transform

import core.*
Expand Down Expand Up @@ -184,23 +185,29 @@ 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)
// can be false for symbols which are defined in some weird combination of supercalls.
|| 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
Expand Down
22 changes: 22 additions & 0 deletions tests/run/i19224.scala
Original file line number Diff line number Diff line change
@@ -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...
}
}

0 comments on commit e1c5589

Please sign in to comment.