SI-6536 Cleanup code around determining accessor requirement

James Iry · James Iry · commit af03afba2596 · 2012-12-13T11:03:13.000-08:00
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
diff --git a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
@@ -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)
diff --git a/src/compiler/scala/tools/nsc/typechecker/SuperAccessors.scala b/src/compiler/scala/tools/nsc/typechecker/SuperAccessors.scala
@@ -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);
@@ -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
     }
 

Original file line number	Diff line number	Diff line change
`@@ -770,7 +770,7 @@ abstract class GenICode extends SubComponent {`
`770`	`770`	`case Apply(fun @ Select(Super(qual, mix), _), args) =>`
`771`	`771`
`772`	`772`	`if (!qual.isInstanceOf[This]) {`
`773`		`- log("WARNING: super call where selector isn't 'this'. May generate invalid bytecode. Previous phases should have tansformed away this form of super.")`
	`773`	`+ log("WARNING: super call where selector isn't 'this'. May generate invalid bytecode. Previous phases should have transformed away this form of super.")`
`774`	`774`	`}`
`775`	`775`	`if (settings.debug.value)`
`776`	`776`	`log("Call to super: " + tree)`