Skip to content

Commit 18201cc

Browse files
oderskyWojciechMazur
authored andcommitted
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 [Cherry-picked 22a959a]
1 parent 1cf11ec commit 18201cc

File tree

2 files changed

+34
-5
lines changed

2 files changed

+34
-5
lines changed

compiler/src/dotty/tools/dotc/transform/Dependencies.scala

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
package dotty.tools.dotc
1+
package dotty.tools
2+
package dotc
23
package transform
34

45
import core.*
@@ -183,23 +184,29 @@ abstract class Dependencies(root: ast.tpd.Tree, @constructorOnly rootContext: Co
183184
def setLogicOwner(local: Symbol) =
184185
val encClass = local.owner.enclosingClass
185186
val preferEncClass =
186-
(
187187
encClass.isStatic
188188
// non-static classes can capture owners, so should be avoided
189189
&& (encClass.isProperlyContainedIn(local.topLevelClass)
190190
// can be false for symbols which are defined in some weird combination of supercalls.
191191
|| encClass.is(ModuleClass, butNot = Package)
192192
// needed to not cause deadlocks in classloader. see t5375.scala
193193
)
194-
)
195-
|| (
194+
&& (!sym.isAnonymousFunction || sym.owner.ownersIterator.exists(_.isConstructor))
195+
// For anonymous functions in static objects, we prefer them to be static because
196+
// that means lambdas are memoized and can be serialized even if the enclosing object
197+
// is not serializable. See run/lambda-serialization-gc.scala and run/i19224.scala.
198+
// On the other hand, we don't want to lift anonymous functions from inside the
199+
// object constructor to be static since that can cause deadlocks by its interaction
200+
// with class initialization. See run/deadlock.scala, which works in Scala 3
201+
// but deadlocks in Scala 2.
202+
||
196203
/* Scala.js: Never move any member beyond the boundary of a DynamicImportThunk.
197204
* DynamicImportThunk subclasses are boundaries between the eventual ES modules
198205
* that can be dynamically loaded. Moving members across that boundary changes
199206
* the dynamic and static dependencies between ES modules, which is forbidden.
200207
*/
201208
ctx.settings.scalajs.value && encClass.isSubClass(jsdefn.DynamicImportThunkClass)
202-
)
209+
203210
logicOwner(sym) = if preferEncClass then encClass else local.enclosingPackageClass
204211

205212
tree match

tests/run/i19224.scala

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// scalajs: --skip
2+
3+
object Test extends App {
4+
val field = 1
5+
def x(): Int => String = (i: Int) => i.toString
6+
def y(): () => String = () => field.toString
7+
8+
locally {
9+
assert(x() == x()) // true on Scala 2, was false on Scala 3...
10+
assert(y() == y()) // also true if `y` accesses object-local fields
11+
12+
def z(): Int => String = (i: Int) => i.toString
13+
assert(z() != z()) // lambdas in constructor are not lifted to static, so no memoization (Scala 2 lifts them, though).
14+
}
15+
16+
val t1 = new C
17+
val t2 = new C
18+
19+
locally {
20+
assert(t1.x() == t2.x()) // true on Scala 2, was false on Scala 3...
21+
}
22+
}

0 commit comments

Comments
 (0)