8000 Don't emit read of omitted `private[this] var` field · lrytz/scala@084ed16 · GitHub
[go: up one dir, main page]
Skip to content

Commit 084ed16

Browse files
lrytzlrytz
committed
Don't emit read of omitted private[this] var field
This is not trivial to fix unfortunately. The `constructors` phase creates the primary constructor and moves statements from the class body into it. While doing that, it rewrites reads of to class parameter fields to the constructor parameters, except for variables - otherwise writes to the variable are not reflected (this is the fix done in scala#7688). Only after that, the phase checks which `private[this]` fields are not used. The implementation for that uses the triage of the class body elements (primary constr, aux constrs, other defs) done in the first step. So if we remove the `private[this] var` field in the second step (because it's never written, and never read outside of the constructor), the constructor ends up with an access to a non-existing field. The fix here delays rewriting reads of class parameter fields to constructor parameters until after the omittable fields are computed.
1 parent 7dbf0ad commit 084ed16

File tree

3 files changed

+43
-23
lines changed

3 files changed

+43
-23
lines changed

‎src/compiler/scala/tools/nsc/transform/Constructors.scala

Lines changed: 35 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,17 @@ abstract class Constructors extends Statics with Transform with TypingTransforme
488488
}
489489

490490
def primaryConstrParams = _primaryConstrParams
491-
def usesSpecializedField = intoConstructor.usesSpecializedField
491+
492+
/*
493+
* `usesSpecializedField` makes a difference in deciding whether constructor-statements
494+
* should be guarded in a `guardSpecializedFieldInit` class, ie in a class that's the generic super-class of
495+
* one or more specialized sub-classes.
496+
*
497+
* Given that `usesSpecializedField` isn't read for any other purpose than the one described above,
498+
* we skip setting `usesSpecializedField` in case the current class isn't `guardSpecializedFieldInit` to start with.
499+
* That way, trips to a map in `specializeTypes` are saved.
500+
*/
501+
var usesSpecializedField: Boolean = false
492502

493503
// The constructor parameter corresponding to an accessor
494504
def parameter(acc: Symbol): Symbol = parameterNamed(acc.unexp 8000 andedName.getterName)
@@ -505,27 +515,16 @@ abstract class Constructors extends Statics with Transform with TypingTransforme
505515
}
506516

507517
// A transformer for expressions that go into the constructor
508-
object intoConstructor extends Transformer {
509-
/*
510-
* `usesSpecializedField` makes a difference in deciding whether constructor-statements
511-
* should be guarded in a `guardSpecializedFieldInit` class, ie in a class that's the generic super-class of
512-
* one or more specialized sub-classes.
513-
*
514-
* Given that `usesSpecializedField` isn't read for any other purpose than the one described above,
515-
* we skip setting `usesSpecializedField` in case the current class isn't `guardSpecializedFieldInit` to start with.
516-
* That way, trips to a map in `specializeTypes` are saved.
517-
*/
518-
var usesSpecializedField: Boolean = false
519-
518+
class IntoConstructor(omittable: Set[Symbol]) extends Transformer {
520519
private def isParamRef(sym: Symbol) = sym.isParamAccessor && sym.owner == clazz
521520

522521
// Terminology: a stationary location is never written after being read.
523-
private def isStationaryParamRef(sym: Symbol) = (
522+
private def isStationaryParamRef(sym: Symbol) = {
524523
isParamRef(sym) &&
525524
!(sym.isGetter && sym.accessed.isVariable) &&
526525
!sym.isSetter &&
527-
!sym.isVariable
528-
)
526+
(!sym.isVariable || omittable(sym))
527+
}
529528

530529
/*
531530
* whether `sym` denotes a param-accessor (ie in a class a PARAMACCESSOR field, or in a trait a method with same flag)
@@ -612,8 +611,8 @@ abstract class Constructors extends Statics with Transform with TypingTransforme
612611

613612
val defs = defBuf.toList
614613
val auxConstructors = auxConstructorBuf.toList
615-
val constructorPrefix = constrPrefixBuf.toList
616-
val constructorStats = constrStatBuf.toList
614+
var constructorPrefix = constrPrefixBuf.toList
615+
var constructorStats = constrStatBuf.toList
617616
val classInitStats = classInitStatBuf.toList
618617

619618
private def triage() = {
@@ -651,8 +650,7 @@ abstract class Constructors extends Statics with Transform with TypingTransforme
651650
def moveEffectToCtor(mods: Modifiers, rhs: Tree, assignSym: Symbol): Unit = {
652651
val initializingRhs =
653652
if ((assignSym eq NoSymbol) || statSym.isLazy) EmptyTree // not memoized, or effect delayed (for lazy val)
654-
else if (!mods.hasStaticFlag) intoConstructor(statSym, primaryConstrSym)(rhs)
655-
else rhs
653+
else rhs.updateAttachment(SymAtt(statSym))
656654

657655
if (initializingRhs ne EmptyTree) {
658656
val initPhase =
@@ -704,10 +702,24 @@ abstract class Constructors extends Statics with Transform with TypingTransforme
704702

705703
// all other statements go into the constructor
706704
case _ =>
707-
constrStatBuf += intoConstructor(impl.symbol, primaryConstrSym)(stat)
705+
constrStatBuf += stat.updateAttachment(SymAtt(impl.symbol))
708706
}
709707
}
710708
}
709+
710+
case class SymAtt(sym: Symbol)
711+
712+
8000 def rewriteFieldAccesses(omittable: Set[Symbol]): Unit = {
713+
val trans = new IntoConstructor(omittable)
714+
val into = (tree: Tree) => tree.getAndRemoveAttachment[SymAtt] match {
715+
case Some(statSym) =>
716+
trans(statSym.sym, primaryConstr.symbol)(tree)
717+
case _ =>
718+
tree
719+
}
720+
constructorPrefix = constructorPrefix.mapConserve(into)
721+
constructorStats = constructorStats.mapConserve(into)
722+
}
711723
}
712724

713725
def transformed = {
@@ -718,6 +730,8 @@ abstract class Constructors extends Statics with Transform with TypingTransforme
718730
if (isDelayedInitSubclass) Set.empty
719731
else computeOmittableAccessors(clazz, defs, auxConstructors, constructorStats)
720732

733+
triage.rewriteFieldAccesses(omittableAccessor)
734+
721735
// TODO: this should omit fields for non-memoized (constant-typed, unit-typed vals need no storage --
722736
// all the action is in the getter)
723737
def omittableSym(sym: Symbol) = omittableAccessor(sym)

‎test/files/run/t12002.check

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
good boy!
2+
27

‎test/files/run/t12002.scala

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,14 @@ class C(private[this] var c: String) {
66
override def toString = x
77
}
88

9+
class D(private[this] var a: Int) {
10+
println(a)
11+
}
12+
913
object Test {
10-
def main(args: Array[String]) = println {
11-
new C("bad")
14+
def main(args: Array[String]): Unit = {
15+
println(new C("bad"))
16+
new D(27)
1217
}
1318
}
1419

0 commit comments

Comments
 (0)
0