Integrating feedback from martin and iulian int...

Integrating feedback from martin and iulian into recent patches and
prettifying checker output. No review.

5 files changed, 57 insertions, 27 deletions

diff --git a/src/compiler/scala/tools/nsc/ast/TreeDSL.scala b/src/compiler/scala/tools/nsc/ast/TreeDSL.scala
index 3b7d4aa26d..fa443447aa 100644
--- a/src/compiler/scala/tools/nsc/ast/TreeDSL.scala
+++ b/src/compiler/scala/tools/nsc/ast/TreeDSL.scala
@@ -193,7 +193,7 @@ trait TreeDSL {
       def defaultPos  = sym.pos
 
       final def ===(rhs: Tree): ResultTreeType =
-        atPos(sym.pos)(mkTree(rhs) setSymbol sym)
+        atPos(pos)(mkTree(rhs) setSymbol sym)
     }
     trait ValCreator {
       self: VODDStart =>
@@ -238,9 +238,9 @@ trait TreeDSL {
     }
 
     class IfStart(cond: Tree, thenp: Tree) {
-      def THEN(x: Tree) = new IfStart(cond, x)
+      def THEN(x: Tree)     = new IfStart(cond, x)
       def ELSE(elsep: Tree) = If(cond, thenp, elsep)
-      def ENDIF = If(cond, thenp, EmptyTree)
+      def ENDIF             = If(cond, thenp, EmptyTree)
     }
     class TryStart(body: Tree, catches: List[CaseDef], fin: Tree) {
       def CATCH(xs: CaseDef*) = new TryStart(body, xs.toList, fin)
@@ -301,9 +301,7 @@ trait TreeDSL {
     def TRY(tree: Tree)   = new TryStart(tree, Nil, EmptyTree)
     def BLOCK(xs: Tree*)  = Block(xs.init.toList, xs.last)
     def NOT(tree: Tree)   = Select(tree, getMember(BooleanClass, nme.UNARY_!))
-
-    private val _SOME     = scalaDot(nme.Some)
-    def SOME(xs: Tree*)   = Apply(_SOME, List(makeTupleTerm(xs.toList, true)))
+    def SOME(xs: Tree*)   = Apply(scalaDot(nme.Some), List(makeTupleTerm(xs.toList, true)))
 
     /** Typed trees from symbols. */
     def THIS(sym: Symbol)             = gen.mkAttributedThis(sym)
diff --git a/src/compiler/scala/tools/nsc/backend/icode/Checkers.scala b/src/compiler/scala/tools/nsc/backend/icode/Checkers.scala
index 8f402876bb..b4a5eca03c 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/Checkers.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/Checkers.scala
@@ -65,6 +65,7 @@ abstract class Checkers {
     val emptyStack = new TypeStack()
 
     val STRING        = REFERENCE(definitions.StringClass)
+    val BOXED_UNIT    = REFERENCE(definitions.BoxedUnitClass)
     val SCALA_NOTHING = REFERENCE(definitions.NothingClass)
     val SCALA_NULL    = REFERENCE(definitions.NullClass)
 
@@ -150,27 +151,52 @@ abstract class Checkers {
     def meet(bl: BasicBlock) {
       val preds = bl.predecessors
 
+      /** XXX workaround #1: one stack empty, the other has BoxedUnit.
+       *  One example where this arises is:
+       *
+       *  def f(b: Boolean): Unit = synchronized { if (b) () }
+       */
+      def isAllUnits(s1: TypeStack, s2: TypeStack) = {
+        List(s1, s2) forall (x => x.types forall (_ == BOXED_UNIT))
+      }
+      /** XXX workaround #2: different stacks heading into an exception
+       *  handler which will clear them anyway.  Examples where it arises:
+       *
+       *  var bippy: Int = synchronized { if (b) 5 else 10 }
+       */
+      def isHandlerBlock() = bl.exceptionHandlerStart
+
+      /** The presence of emptyStack means that path has not yet been checked
+       *  (and may not be empty) thus the reference eq tests.
+       */
       def meet2(s1: TypeStack, s2: TypeStack): TypeStack = {
+        def workaround(msg: String) = {
+          checkerDebug(msg + ": " + method + " at block " + bl)
+          checkerDebug("  s1: " + s1)
+          checkerDebug("  s2: " + s2)
+          new TypeStack()
+        }
+
         if (s1 eq emptyStack) s2
         else if (s2 eq emptyStack) s1
-        else {
-          if (s1.isEmpty && s2.isEmpty) {
-            // PP: I do not know the significance of this condition, but it arises a lot
-            // so I'm taking the intuitive position that any two empty stacks are as good
-            // as another, rather than throwing an exception as it did.
-            // If the reference eq test is achieving something please document.
-            return emptyStack
-          }
-          else if (s1.length != s2.length)
+        else if (s1.length != s2.length) {
+          if (isAllUnits(s1, s2))
+            workaround("Ignoring mismatched boxed units")
+          else if (isHandlerBlock)
+            workaround("Ignoring mismatched stacks entering exception handler")
+          else
             throw new CheckerException("Incompatible stacks: " + s1 + " and " + s2 + " in " + method + " at entry to block: " + bl);
+        }
+        else {
+          val newStack = new TypeStack((s1.types, s2.types).zipped map lub)
+          if (newStack.nonEmpty)
+            checkerDebug("Checker created new stack:\n  (%s, %s) => %s".format(s1, s2, newStack))
 
-          val types = (s1.types, s2.types).zipped map lub
-          checkerDebug("Checker created new stack: (%s, %s) => %s".format(s1.types, s2.types, types))
-          new TypeStack(types)
+          newStack
         }
       }
 
-      if (preds != Nil) {
+      if (preds.nonEmpty) {
         in(bl) = (preds map out.apply) reduceLeft meet2;
         log("Input changed for block: " + bl +" to: " + in(bl));
       }
@@ -221,8 +247,13 @@ abstract class Checkers {
 
       def pushStack(xs: TypeKind*): Unit = {
         xs foreach { x =>
-          if (x == UNIT)
+          if (x == UNIT) {
+            /** PP: I admit I'm not yet figuring out how the stacks will balance when
+             *  we ignore UNIT, but I expect this knowledge will emerge naturally.
+             *  In the meantime I'm logging it to help me out.
+             */
             logChecker("Ignoring pushed UNIT")
+          }
           else {
             stack push x
             checkerDebug(sizeString(true) + x)
diff --git a/src/compiler/scala/tools/nsc/backend/icode/TypeKinds.scala b/src/compiler/scala/tools/nsc/backend/icode/TypeKinds.scala
index b18c0da2a9..e72db4abc2 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/TypeKinds.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/TypeKinds.scala
@@ -246,6 +246,7 @@ trait TypeKinds { self: ICodes =>
 
   /** A class type. */
   final case class REFERENCE(cls: Symbol) extends TypeKind {
+    override def toString = "REF(" + cls + ")"
     assert(cls ne null,
            "REFERENCE to null class symbol.")
     assert(cls != ArrayClass,
diff --git a/src/compiler/scala/tools/nsc/backend/icode/TypeStacks.scala b/src/compiler/scala/tools/nsc/backend/icode/TypeStacks.scala
index 74abec78bc..35fad7db87 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/TypeStacks.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/TypeStacks.scala
@@ -24,7 +24,7 @@ trait TypeStacks {
 
   class TypeStack(var types: Rep) {
     if (types.nonEmpty)
-      checkerDebug("TypeStack init: " + types.mkString(", "))
+      checkerDebug("Created " + this)
 
     def this() = this(Nil)
     def this(that: TypeStack) = this(that.types)
@@ -79,8 +79,8 @@ trait TypeStacks {
 
     /* This method returns a String representation of the stack */
     override def toString() =
-      if (types.isEmpty) "TypeStack()"
-      else types.mkString("TypeStack(" + types.size + " elems) {\n  ", "\n  ", "\n}")
+      if (types.isEmpty) "[]"
+      else types.mkString("[", " ", "]")
 
     override def hashCode() = types.hashCode()
     override def equals(other: Any): Boolean = other match {
diff --git a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala
index e65965d3ff..160a0d305e 100644
--- a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala
+++ b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala
@@ -316,8 +316,8 @@ abstract class ExplicitOuter extends InfoTransform
       /** If we don't re-type the tree, we see self-type related crashes like #266.
        */
       localTyper typed {
-        (DEF(outerAcc) withType null) === rhs
-      } setPos currentClass.pos
+        (DEF(outerAcc) withPos currentClass.pos withType null) === rhs
+      }
     }
 
     /** The definition tree of the outer accessor for class mixinClass.
@@ -334,11 +334,11 @@ abstract class ExplicitOuter extends InfoTransform
         else gen.mkAttributedQualifier(currentClass.thisType baseType mixinClass prefix)
 
       localTyper typed {
-        DEF(outerAcc) === {
+        (DEF(outerAcc) withPos currentClass.pos) === {
           // Need to cast for nested outer refs in presence of self-types. See ticket #3274.
           transformer.transform(path) AS_ANY outerAcc.info.resultType
         }
-      } setPos currentClass.pos
+      }
     }
 
     /** If FLAG is set on symbol, sets notFLAG (this exists in anticipation of generalizing). */