Skip to content

Commit ee84531

Browse files
committed
Handle cycles in early promotion
1 parent 48f7b97 commit ee84531

File tree

4 files changed

+73
-13
lines changed

4 files changed

+73
-13
lines changed

compiler/src/dotty/tools/dotc/transform/init/Semantic.scala

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ class Semantic {
6060
*/
6161
case class Objekt(val fields: mutable.Map[Symbol, Value]) {
6262
var allFieldsInitialized: Boolean = false
63+
64+
val promotedValues = mutable.Set.empty[Value]
6365
}
6466

6567
/** Abstract heap stores abstract objects
@@ -219,7 +221,7 @@ class Semantic {
219221
if target.isPrimaryConstructor then
220222
val cls = target.owner.asClass
221223
val tpl = cls.defTree.asInstanceOf[TypeDef].rhs.asInstanceOf[Template]
222-
eval(tpl, thisRef, cls, cacheResult = true)
224+
eval(tpl, thisRef, cls, cacheResult = true)(using heap, ctx, trace.add(tpl))
223225
else
224226
val rhs = target.defTree.asInstanceOf[ValOrDefDef].rhs
225227
eval(rhs, thisRef, target.owner.asClass, cacheResult = true)
@@ -317,19 +319,22 @@ class Semantic {
317319
case Cold => false
318320

319321
case warm: Warm =>
320-
warm.outer.canDirectlyPromote
322+
heap.promotedValues.contains(warm)
323+
|| warm.outer.canDirectlyPromote
321324

322325
case thisRef: ThisRef =>
323-
heap.allFieldsInitialized || {
326+
heap.promotedValues.contains(thisRef) || {
324327
// If we have all fields initialized, then we can promote This to hot.
325-
heap.allFieldsInitialized = thisRef.klass.appliedRef.fields.forall { denot =>
328+
val allFieldsInitialized = thisRef.klass.appliedRef.fields.forall { denot =>
326329
val sym = denot.symbol
327330
sym.isOneOf(Flags.Lazy | Flags.Deferred) || heap.fields.contains(sym)
328331
}
329-
heap.allFieldsInitialized
332+
if allFieldsInitialized then heap.promotedValues += thisRef
333+
allFieldsInitialized
330334
}
331335

332-
case fun: Fun => false
336+
case fun: Fun =>
337+
heap.promotedValues.contains(fun)
333338

334339
case RefSet(refs) =>
335340
refs.forall(_.canDirectlyPromote)
@@ -348,8 +353,13 @@ class Semantic {
348353
else PromoteThis(source, trace) :: Nil
349354

350355
case warm: Warm =>
351-
if warm.outer.canDirectlyPromote then Nil
352-
else warm.tryPromote(msg, source)
356+
if warm.canDirectlyPromote then Nil
357+
else {
358+
heap.promotedValues += warm
359+
val errors = warm.tryPromote(msg, source)
360+
if errors.nonEmpty then heap.promotedValues -= warm
361+
errors
362+
}
353363

354364
case Fun(body, thisV, klass) =>
355365
val res = eval(body, thisV, klass)
@@ -374,6 +384,9 @@ class Semantic {
374384
* 2. for each concrete field `f` of the warm object:
375385
* promote the field value
376386
*
387+
* TODO: we need to revisit whether this is needed once make the
388+
* system more flexible in other dimentions: e.g. leak to
389+
* methods or constructors, or use ownership for creating cold data structures.
377390
*/
378391
def tryPromote(msg: String, source: Tree): Contextual[List[Error]] = log("promote " + warm.show, printer) {
379392
val classRef = warm.klass.appliedRef
@@ -388,15 +401,15 @@ class Semantic {
388401
val f = denot.symbol
389402
if !f.isOneOf(Flags.Deferred | Flags.Private | Flags.Protected) && f.hasSource then
390403
val res = warm.select(f, source)
391-
buffer ++= res.ensureHot(msg, source).errors
404+
buffer ++= res.ensureHot(msg, source)(using heap, ctx, trace.add(f.defTree)).errors
392405
buffer.nonEmpty
393406
}
394407

395408
buffer.nonEmpty || methods.exists { denot =>
396409
val m = denot.symbol
397410
if !m.isConstructor && m.hasSource then
398-
val res = warm.call(m, superType = NoType, source = source)
399-
buffer ++= res.ensureHot(msg, source).errors
411+
val res = warm.call(m, superType = NoType, source = source)(using heap, ctx, trace.add(m.defTree))
412+
buffer ++= res.ensureHot(msg, source)(using heap, ctx, trace.add(m.defTree)).errors
400413
buffer.nonEmpty
401414
}
402415

@@ -480,7 +493,7 @@ class Semantic {
480493

481494
val cls = tref.classSymbol.asClass
482495
val res = outerValue(tref, thisV, klass, tpt)
483-
(res ++ errors).instantiate(cls, ctor, expr)
496+
(res ++ errors).instantiate(cls, ctor, expr)(using heap, ctx, trace.add(expr))
484497

485498
case Call(ref, argss) =>
486499
// check args
@@ -705,7 +718,7 @@ class Semantic {
705718

706719
// follow constructor
707720
if !cls.defTree.isEmpty then
708-
val res2 = thisV.call(ctor, superType = NoType, source)
721+
val res2 = thisV.call(ctor, superType = NoType, source)(using heap, ctx, trace.add(source))
709722
errorBuffer ++= res2.errors
710723

711724
// parents

tests/init/neg/inner-loop.scala

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,22 @@ class Outer2 { outer =>
1313
val inner = new Inner
1414
val n = 6
1515
}
16+
17+
class Test {
18+
class Outer3 { outer =>
19+
class Inner extends Outer3 {
20+
val x = 5 + n
21+
}
22+
val inner = new Inner
23+
val n = 6
24+
}
25+
26+
val outer = new Outer3
27+
// Warm objects with inner classes not checked.
28+
// If we change policy to check more eagerly,
29+
// the check has to avoid loop here.
30+
31+
println(outer) // error
32+
33+
val m = 10
34+
}

tests/init/neg/promotion-loop.check

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
-- Error: tests/init/neg/promotion-loop.scala:16:10 --------------------------------------------------------------------
2+
16 | println(b) // error
3+
| ^
4+
| Promoting the value to fully-initialized is unsafe.
5+
|
6+
| The unsafe promotion may cause the following problem(s):
7+
|
8+
| 1. Promote the value under initialization to fully-initialized. Calling trace:
9+
| -> val outer = test [ promotion-loop.scala:12 ]

tests/init/neg/promotion-loop.scala

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
class Test { test =>
2+
class A {
3+
val self = this
4+
}
5+
6+
val a = new A
7+
println(a)
8+
9+
10+
class B {
11+
val self = this
12+
val outer = test
13+
}
14+
15+
val b = new B
16+
println(b) // error
17+
18+
val n = 10
19+
}

0 commit comments

Comments
 (0)