Skip to content

Commit

Permalink
SI-6536 Cleanup code around determining accessor requirement
Browse files Browse the repository at this point in the history
Moves the logic for the condition on requiring an accessor into a local method
with a match to make thing cleaner. Also determines the mix type using
a pattern match that logs but does not fail if the symbol isn't of the expected
type. Finally fixes a typo in an assertion in GenICode.

review @paulp
  • Loading branch information
James Iry committed Dec 13, 2012
1 parent 2124b9d commit af03afb
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 12 deletions.
2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
Expand Up @@ -770,7 +770,7 @@ abstract class GenICode extends SubComponent {
case Apply(fun @ Select(Super(qual, mix), _), args) =>

if (!qual.isInstanceOf[This]) {
log("WARNING: super call where selector isn't 'this'. May generate invalid bytecode. Previous phases should have tansformed away this form of super.")
log("WARNING: super call where selector isn't 'this'. May generate invalid bytecode. Previous phases should have transformed away this form of super.")
}
if (settings.debug.value)
log("Call to super: " + tree)
Expand Down
36 changes: 25 additions & 11 deletions src/compiler/scala/tools/nsc/typechecker/SuperAccessors.scala
Expand Up @@ -110,7 +110,6 @@ abstract class SuperAccessors extends transform.Transform with transform.TypingT
val Select(sup @ Super(_, mix), name) = sel
val sym = sel.symbol
val clazz = sup.symbol
val SuperType(_, mixtpe) = sup.tpe

if (sym.isDeferred) {
val member = sym.overridingSymbol(clazz);
Expand All @@ -120,20 +119,35 @@ abstract class SuperAccessors extends transform.Transform with transform.TypingT
"unless it is overridden by a member declared `abstract' and `override'");
}

// determine if the mix in clazz.super[mix].name is a trait
def mixTpeIsTrait = sup.tpe match {
case SuperType(_, mixTpe) => mixTpe.typeSymbol.isTrait
case _ =>
log("Warning: could not determine the type of mix " + mix + " by going through a Super node's "+
"type because instead of a SuperType it was " + sup.tpe)
false
}

// we need an accessor to get to a super on an outer thing, but only if we can't call name more directly on
// a trait implementation class. So this complicated condition is leaving alone cases where we don't need to do
// anything special (i.e. we're getting a direct super class) or where a later transform will inject a call to
// a trait implementation method directly.
// So, we're looking for items of the form clazz.super[mix].name (or clazz.super.name wich is seen as clazz.super[EMPTY].name.
// with some limitations. First, name has to be a term rather than a type.
// Then there are a couple of cases. If mix is empty then we only need an accessor if clazz is a trait, it's not this current class,
// or the validCurentOwner setting is false...which...ugh, is a mess.
// If the mix is set then if it refers to a class and the clazz part isn't the current class
// it's not just super[mix].name then we need to generate an accessor.
// SI-6536 has more discussion about how this works.
if (name.isTermName && (mix == tpnme.EMPTY && (clazz.isTrait || clazz != currentClass || !validCurrentOwner ))
|| (mix != tpnme.EMPTY && !mixtpe.typeSymbol.isTrait && clazz != currentClass))
ensureAccessor(sel)
//
// SI-6536 has more discussion about how this works.
//
// So, we're looking for items of the form clazz.super[mix].name (or clazz.super.name wich is seen as
// clazz.super[EMPTY].name with some limitations. First, name has to be a term rather than a type.
// Then there are a couple of cases.
def requiresAccessor = name.isTermName && (mix match {
// If mix is empty then we only need an accessor if clazz is a trait, it's not this current class,
// or the validCurentOwner setting is false...which...ugh, is a mess.
case tpnme.EMPTY => clazz.isTrait || clazz != currentClass || !validCurrentOwner
// If the mix is set then if it refers to a class and the clazz part isn't the current class
// it's not just super[mix].name then we need to generate an accessor.
case _ => clazz != currentClass && !mixTpeIsTrait
})

if (requiresAccessor) ensureAccessor(sel)
else sel
}

Expand Down

0 comments on commit af03afb

Please sign in to comment.