Skip to content

Commit 37e7e2a

Browse files
committed
Fix crash by using special resolveEnv function for local read and write
1 parent f6d6b17 commit 37e7e2a

File tree

3 files changed

+51
-15
lines changed

3 files changed

+51
-15
lines changed

Diff for: compiler/src/dotty/tools/dotc/transform/init/Objects.scala

+47-11
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,39 @@ class Objects(using Context @constructorOnly):
436436
case _ =>
437437
throw new RuntimeException("Incorrect local environment for initializing " + x.show)
438438

439+
/**
440+
* Resolve the environment by searching for a given symbol.
441+
*
442+
* Searches for the environment that owns `target`, starting from `env` as the innermost.
443+
*
444+
* Due to widening, the corresponding environment might not exist. As a result reading the local
445+
* variable will return `Cold` and it's forbidden to write to the local variable.
446+
*
447+
* @param target The symbol to search for.
448+
* @param thisV The value for `this` of the enclosing class where the local variable is referenced.
449+
* @param env The local environment where the local variable is referenced.
450+
*
451+
* @return the environment that owns the `target` and value for `this` owned by the given method.
452+
*/
453+
def resolveEnvByValue(target: Symbol, thisV: ThisValue, env: Data)(using Context): Option[(ThisValue, Data)] = log("Resolving env by value for " + target.show + ", this = " + thisV.show + ", env = " + env.show, printer) {
454+
env match
455+
case localEnv: LocalEnv =>
456+
if localEnv.getVal(target).isDefined then Some(thisV -> localEnv)
457+
else if localEnv.getVar(target).isDefined then Some(thisV -> localEnv)
458+
else resolveEnvByValue(target, thisV, localEnv.outer)
459+
case NoEnv =>
460+
thisV match
461+
case ref: OfClass =>
462+
ref.outer match
463+
case outer : ThisValue =>
464+
resolveEnvByValue(target, outer, ref.env)
465+
case _ =>
466+
// TODO: properly handle the case where ref.outer is ValueSet
467+
None
468+
case _ =>
469+
None
470+
}
471+
439472
/**
440473
* Resolve the environment owned by the given method.
441474
*
@@ -451,17 +484,17 @@ class Objects(using Context @constructorOnly):
451484
*
452485
* @return the environment and value for `this` owned by the given method.
453486
*/
454-
def resolveEnv(meth: Symbol, thisV: ThisValue, env: Data)(using Context): Option[(ThisValue, Data)] = log("Resolving env for " + meth.show + ", this = " + thisV.show + ", env = " + env.show, printer) {
487+
def resolveEnvByOwner(meth: Symbol, thisV: ThisValue, env: Data)(using Context): Option[(ThisValue, Data)] = log("Resolving env by owner for " + meth.show + ", this = " + thisV.show + ", env = " + env.show, printer) {
455488
env match
456489
case localEnv: LocalEnv =>
457490
if localEnv.meth == meth then Some(thisV -> env)
458-
else resolveEnv(meth, thisV, localEnv.outer)
491+
else resolveEnvByOwner(meth, thisV, localEnv.outer)
459492
case NoEnv =>
460493
thisV match
461494
case ref: OfClass =>
462495
ref.outer match
463496
case outer : ThisValue =>
464-
resolveEnv(meth, outer, ref.env)
497+
resolveEnvByOwner(meth, outer, ref.env)
465498
case _ =>
466499
// TODO: properly handle the case where ref.outer is ValueSet
467500
None
@@ -724,7 +757,7 @@ class Objects(using Context @constructorOnly):
724757
if meth.owner.isClass then
725758
(ref, Env.NoEnv)
726759
else
727-
Env.resolveEnv(meth.owner.enclosingMethod, ref, summon[Env.Data]).getOrElse(Cold -> Env.NoEnv)
760+
Env.resolveEnvByOwner(meth.owner.enclosingMethod, ref, summon[Env.Data]).getOrElse(Cold -> Env.NoEnv)
728761

729762
val env2 = Env.ofDefDef(ddef, args.map(_.value), outerEnv)
730763
extendTrace(ddef) {
@@ -771,9 +804,9 @@ class Objects(using Context @constructorOnly):
771804
end if
772805

773806
case _ =>
774-
// by-name closure
775-
given Env.Data = env
776-
extendTrace(code) { eval(code, thisV, klass, cacheResult = true) }
807+
// Should be unreachable, by-name closures are handled by readLocal
808+
report.warning("[Internal error] Only DefDef should be possible here, but found " + code.show + ". " + Trace.show, Trace.position)
809+
Bottom
777810

778811
case ValueSet(vs) =>
779812
vs.map(v => call(v, meth, args, receiver, superType)).join
@@ -962,7 +995,7 @@ class Objects(using Context @constructorOnly):
962995
(thisV.widenRefOrCold(1), Env.NoEnv)
963996
else
964997
// klass.enclosingMethod returns its primary constructor
965-
Env.resolveEnv(klass.owner.enclosingMethod, thisV, summon[Env.Data]).getOrElse(Cold -> Env.NoEnv)
998+
Env.resolveEnvByOwner(klass.owner.enclosingMethod, thisV, summon[Env.Data]).getOrElse(Cold -> Env.NoEnv)
966999

9671000
val instance = OfClass(klass, outerWidened, ctor, args.map(_.value), envWidened)
9681001
callConstructor(instance, ctor, args)
@@ -992,7 +1025,9 @@ class Objects(using Context @constructorOnly):
9921025
*/
9931026
def readLocal(thisV: ThisValue, sym: Symbol): Contextual[Value] = log("reading local " + sym.show, printer, (_: Value).show) {
9941027
def isByNameParam(sym: Symbol) = sym.is(Flags.Param) && sym.info.isInstanceOf[ExprType]
995-
Env.resolveEnv(sym.enclosingMethod, thisV, summon[Env.Data]) match
1028+
// Can't use enclosingMethod here because values defined in a by-name closure will have the wrong enclosingMethod,
1029+
// since our phase is before elimByName.
1030+
Env.resolveEnvByValue(sym, thisV, summon[Env.Data]) match
9961031
case Some(thisV -> env) =>
9971032
if sym.is(Flags.Mutable) then
9981033
// Assume forward reference check is doing a good job
@@ -1047,8 +1082,9 @@ class Objects(using Context @constructorOnly):
10471082
*/
10481083
def writeLocal(thisV: ThisValue, sym: Symbol, value: Value): Contextual[Value] = log("write local " + sym.show + " with " + value.show, printer, (_: Value).show) {
10491084
assert(sym.is(Flags.Mutable), "Writing to immutable variable " + sym.show)
1050-
1051-
Env.resolveEnv(sym.enclosingMethod, thisV, summon[Env.Data]) match
1085+
// Can't use enclosingMethod here because values defined in a by-name closure will have the wrong enclosingMethod,
1086+
// since our phase is before elimByName.
1087+
Env.resolveEnvByValue(sym, thisV, summon[Env.Data]) match
10521088
case Some(thisV -> env) =>
10531089
given Env.Data = env
10541090
Env.getVar(sym) match

Diff for: tests/init/pos/byname.scala renamed to tests/init-global/pos/byname.scala

+3-3
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@ class A extends T:
55
override def bar(i: => Int): Int = i + 1
66

77
class B extends T:
8-
override def bar(i: => Int): Int = i + 2
8+
override def bar(i: => Int): Int = i
99

1010
object A:
1111
val a: T = if ??? then new A else new B
12-
def foo(b: List[Int]) = a.bar(b match {
13-
case x :: xs => 1
12+
def foo(b: List[Int]): Int = a.bar(b match {
13+
case head :: rest => head + foo(rest) + a.bar(head)
1414
case Nil => 0
1515
})
1616

Diff for: tests/init-global/warn/lazy-local-val.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,5 @@ object B:
1515
lazy val b = a
1616
Box(b)
1717

18-
val box = f(n) // warn
18+
val box = f(n)
1919
val n = 10

0 commit comments

Comments
 (0)