Skip to content

Commit 1064ada

Browse files
authored
Merge pull request #5388 from adriaanm/issue-219
Avoid mismatched symbols in fields phase
2 parents 2a45961 + 5f64ee5 commit 1064ada

File tree

4 files changed

+79
-44
lines changed

4 files changed

+79
-44
lines changed

src/compiler/scala/tools/nsc/transform/Fields.scala

+66-42
Original file line numberDiff line numberDiff line change
@@ -207,9 +207,8 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
207207
moduleVar
208208
}
209209

210-
private def moduleInit(module: Symbol) = {
210+
private def moduleInit(module: Symbol, moduleVar: Symbol) = {
211211
// println(s"moduleInit for $module in ${module.ownerChain} --> ${moduleVarOf.get(module)}")
212-
val moduleVar = moduleOrLazyVarOf(module)
213212
def moduleVarRef = gen.mkAttributedRef(moduleVar)
214213

215214
// for local modules, we synchronize on the owner of the method that owns the module
@@ -238,7 +237,8 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
238237
*/
239238
val computeName = nme.newLazyValSlowComputeName(module.name)
240239
val computeMethod = DefDef(NoMods, computeName, Nil, ListOfNil, TypeTree(UnitTpe), gen.mkSynchronized(monitorHolder)(If(needsInit, init, EmptyTree)))
241-
Block(computeMethod :: If(needsInit, Apply(Ident(computeName), Nil), EmptyTree) :: Nil, moduleVarRef)
240+
Block(computeMethod :: If(needsInit, Apply(Ident(computeName), Nil), EmptyTree) :: Nil,
241+
gen.mkCast(moduleVarRef, module.info.resultType))
242242
}
243243

244244
// NoSymbol for lazy accessor sym with unit result type
@@ -590,62 +590,81 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
590590
}
591591

592592
// synth trees for accessors/fields and trait setters when they are mixed into a class
593-
def fieldsAndAccessors(clazz: Symbol): List[ValOrDefDef] = {
594-
def fieldAccess(accessor: Symbol): List[Tree] = {
595-
val fieldName = accessor.localName
596-
val field = clazz.info.decl(fieldName)
597-
// The `None` result denotes an error, but it's refchecks' job to report it (this fallback is for robustness).
598-
// This is the result of overriding a val with a def, so that no field is found in the subclass.
599-
if (field.exists) List(Select(This(clazz), field))
600-
else Nil
601-
}
602-
603-
def getterBody(getter: Symbol): List[Tree] = {
593+
def fieldsAndAccessors(clazz: Symbol): List[Tree] = {
594+
// scala/scala-dev#219
595+
// Cast to avoid spurious mismatch in paths containing trait vals that have
596+
// not been rebound to accessors in the subclass we're in now.
597+
// For example, for a lazy val mixed into a class, the lazy var's info
598+
// will not refer to symbols created during our info transformer,
599+
// so if its type depends on a val that is now implemented after the info transformer,
600+
// we'll get a mismatch when assigning `rhs` to `lazyVarOf(getter)`.
601+
// TODO: could we rebind more aggressively? consider overriding in type equality?
602+
def cast(tree: Tree, pt: Type) = gen.mkAsInstanceOf(tree, pt)
603+
604+
// Could be NoSymbol, which denotes an error, but it's refchecks' job to report it (this fallback is for robustness).
605+
// This is the result of overriding a val with a def, so that no field is found in the subclass.
606+
def fieldAccess(accessor: Symbol): Symbol =
607+
afterOwnPhase { clazz.info.decl(accessor.localName) }
608+
609+
def getterBody(getter: Symbol): Tree =
604610
// accessor created by newMatchingModuleAccessor for a static module that does need an accessor
605611
// (because there's a matching member in a super class)
606-
if (getter.asTerm.referenced.isModule) {
607-
List(gen.mkAttributedRef(clazz.thisType, getter.asTerm.referenced))
608-
} else {
612+
if (getter.asTerm.referenced.isModule)
613+
mkAccessor(getter)(cast(Select(This(clazz), getter.asTerm.referenced), getter.info.resultType))
614+
else {
609615
val fieldMemoization = fieldMemoizationIn(getter, clazz)
610-
if (fieldMemoization.constantTyped) List(gen.mkAttributedQualifier(fieldMemoization.tp)) // TODO: drop when we no longer care about producing identical bytecode
611-
else fieldAccess(getter)
616+
// TODO: drop getter for constant? (when we no longer care about producing identical bytecode?)
617+
if (fieldMemoization.constantTyped) mkAccessor(getter)(gen.mkAttributedQualifier(fieldMemoization.tp))
618+
else fieldAccess(getter) match {
619+
case NoSymbol => EmptyTree
620+
case fieldSel => mkAccessor(getter)(cast(Select(This(clazz), fieldSel), getter.info.resultType))
621+
}
612622
}
613-
}
614623

615624
// println(s"accessorsAndFieldsNeedingTrees for $templateSym: $accessorsAndFieldsNeedingTrees")
616-
def setterBody(setter: Symbol): List[Tree] = {
625+
def setterBody(setter: Symbol): Tree =
617626
// trait setter in trait
618-
if (clazz.isTrait) List(EmptyTree)
627+
if (clazz.isTrait) mkAccessor(setter)(EmptyTree)
619628
// trait setter for overridden val in class
620-
else if (checkAndClearOverriddenTraitSetter(setter)) List(mkTypedUnit(setter.pos))
629+
else if (checkAndClearOverriddenTraitSetter(setter)) mkAccessor(setter)(mkTypedUnit(setter.pos))
621630
// trait val/var setter mixed into class
622-
else fieldAccess(setter) map (fieldSel => Assign(fieldSel, Ident(setter.firstParam)))
623-
}
631+
else fieldAccess(setter) match {
632+
case NoSymbol => EmptyTree
633+
case fieldSel => afterOwnPhase { // the assign only type checks after our phase (assignment to val)
634+
mkAccessor(setter)(Assign(Select(This(clazz), fieldSel), cast(Ident(setter.firstParam), fieldSel.info)))
635+
}
636+
}
624637

625-
def moduleAccessorBody(module: Symbol): List[Tree] = List(
638+
def moduleAccessorBody(module: Symbol): Tree =
626639
// added during synthFieldsAndAccessors using newModuleAccessor
627640
// a module defined in a trait by definition can't be static (it's a member of the trait and thus gets a new instance for every outer instance)
628-
if (clazz.isTrait) EmptyTree
641+
if (clazz.isTrait) mkAccessor(module)(EmptyTree)
629642
// symbol created by newModuleAccessor for a (non-trait) class
630-
else moduleInit(module)
631-
)
643+
else {
644+
mkAccessor(module)(moduleInit(module, moduleOrLazyVarOf(module)))
645+
}
632646

633647
val synthAccessorInClass = new SynthLazyAccessorsIn(clazz)
634-
def superLazy(getter: Symbol): List[ValOrDefDef] = {
648+
def superLazy(getter: Symbol): Tree = {
635649
assert(!clazz.isTrait)
636-
// this contortion was the only way I can get the super select to be type checked correctly.. TODO: why does SelectSuper not work?
637-
val rhs = Apply(Select(Super(This(clazz), tpnme.EMPTY), getter.name), Nil)
638-
explodeThicket(synthAccessorInClass.expandLazyClassMember(lazyVarOf(getter), getter, rhs, Map.empty)).asInstanceOf[List[ValOrDefDef]]
650+
// this contortion was the only way I can get the super select to be type checked correctly..
651+
// TODO: why does SelectSuper not work?
652+
val selectSuper = Select(Super(This(clazz), tpnme.EMPTY), getter.name)
653+
654+
val lazyVar = lazyVarOf(getter)
655+
val rhs = cast(Apply(selectSuper, Nil), lazyVar.info)
656+
657+
synthAccessorInClass.expandLazyClassMember(lazyVar, getter, rhs, Map.empty)
639658
}
640659

641-
clazz.info.decls.toList.filter(checkAndClearNeedsTrees) flatMap {
642-
case module if module hasAllFlags (MODULE | METHOD) => moduleAccessorBody(module) map mkAccessor(module)
660+
(afterOwnPhase { clazz.info.decls } toList) filter checkAndClearNeedsTrees map {
661+
case module if module hasAllFlags (MODULE | METHOD) => moduleAccessorBody(module)
643662
case getter if getter hasAllFlags (LAZY | METHOD) => superLazy(getter)
644-
case setter if setter.isSetter => setterBody(setter) map mkAccessor(setter)
645-
case getter if getter.hasFlag(ACCESSOR) => getterBody(getter) map mkAccessor(getter)
646-
case field if !(field hasFlag METHOD) => Some(mkTypedValDef(field)) // vals/vars and module vars (cannot have flags PACKAGE | JAVA since those never receive NEEDS_TREES)
647-
case _ => None
648-
}
663+
case setter if setter.isSetter => setterBody(setter)
664+
case getter if getter.hasFlag(ACCESSOR) => getterBody(getter)
665+
case field if !(field hasFlag METHOD) => mkTypedValDef(field) // vals/vars and module vars (cannot have flags PACKAGE | JAVA since those never receive NEEDS_TREES)
666+
case _ => EmptyTree
667+
} filterNot (_ == EmptyTree) // there will likely be many EmptyTrees, but perhaps no thicket blocks that need expanding
649668
}
650669

651670
def rhsAtOwner(stat: ValOrDefDef, newOwner: Symbol): Tree =
@@ -705,7 +724,7 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
705724
if (currOwner.isClass) cd
706725
else { // local module -- symbols cannot be generated by info transformer, so do it all here
707726
val moduleVar = newModuleVarSymbol(currOwner, statSym, statSym.info.resultType)
708-
Thicket(cd :: mkTypedValDef(moduleVar) :: mkAccessor(statSym)(moduleInit(statSym)) :: Nil)
727+
Thicket(cd :: mkTypedValDef(moduleVar) :: mkAccessor(statSym)(moduleInit(statSym, moduleVar)) :: Nil)
709728
}
710729

711730
case tree =>
@@ -724,7 +743,12 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
724743
override def transformStats(stats: List[Tree], exprOwner: Symbol): List[Tree] = {
725744
val addedStats =
726745
if (!currentOwner.isClass || currentOwner.isPackageClass) Nil
727-
else afterOwnPhase { fieldsAndAccessors(currentOwner) }
746+
else {
747+
val thickets = fieldsAndAccessors(currentOwner)
748+
if (thickets exists mustExplodeThicket)
749+
thickets flatMap explodeThicket
750+
else thickets
751+
}
728752

729753
val inRealClass = currentOwner.isClass && !(currentOwner.isPackageClass || currentOwner.isTrait)
730754
if (inRealClass)

test/files/pos/sd219.scala

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
class Global { class Name }
2+
3+
trait CommonPrintUtils {
4+
val global: Global
5+
6+
lazy val precedence: global.Name => Int = ???
7+
}
8+
9+
trait CompilerProvider { val global: Global = ??? }
10+
11+
class AbstractPrinter extends CommonPrintUtils with CompilerProvider

test/files/run/delambdafy_t6028.check

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ package <empty> {
4747
final <stable> private[this] def MethodLocalObject$1(barParam$1: String, MethodLocalObject$module$1: runtime.VolatileObjectRef): T#MethodLocalObject$2.type = {
4848
if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null))
4949
T.this.MethodLocalObject$lzycompute$1(barParam$1, MethodLocalObject$module$1);
50-
MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]()
50+
(MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type](): T#MethodLocalObject$2.type)
5151
};
5252
final <artifact> private[this] def $anonfun$tryy$1(tryyParam$1: String, tryyLocal$1: runtime.ObjectRef): Unit = try {
5353
tryyLocal$1.elem = tryyParam$1

test/files/run/t6028.check

+1-1
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ package <empty> {
5959
final <stable> private[this] def MethodLocalObject$1(barParam$1: Int, MethodLocalObject$module$1: runtime.VolatileObjectRef): T#MethodLocalObject$2.type = {
6060
if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null))
6161
T.this.MethodLocalObject$lzycompute$1(barParam$1, MethodLocalObject$module$1);
62-
MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]()
62+
(MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type](): T#MethodLocalObject$2.type)
6363
};
6464
@SerialVersionUID(value = 0) final <synthetic> class $anonfun$tryy$1 extends scala.runtime.AbstractFunction0$mcV$sp with Serializable {
6565
def <init>($outer: T, tryyParam$1: Int, tryyLocal$1: runtime.IntRef): <$anon: Function0> = {

0 commit comments

Comments
 (0)