Some progress on reviving TreeCheckers, plus a ...

Some progress on reviving TreeCheckers, plus a couple bugfixes and
better error messages revealed by that progress. Also applied tiny
increment in understanding to fixing up TreeDSL some more. The hand of
martin guides from above, so no review.

8 files changed, 137 insertions, 85 deletions

diff --git a/src/compiler/scala/tools/nsc/ast/TreeDSL.scala b/src/compiler/scala/tools/nsc/ast/TreeDSL.scala
index 547476a6db..defe24dec8 100644
--- a/src/compiler/scala/tools/nsc/ast/TreeDSL.scala
+++ b/src/compiler/scala/tools/nsc/ast/TreeDSL.scala
@@ -63,8 +63,6 @@ trait TreeDSL {
     def fn(lhs: Tree, op: Symbol, args: Tree*)  = Apply(Select(lhs, op), args.toList)
 
     class TreeMethods(target: Tree) {
-      private def toAnyRef(x: Tree) = x setType AnyRefClass.tpe
-
       /** logical/comparison ops **/
       def OR(other: Tree) =
         if (target == EmptyTree) other
@@ -87,8 +85,7 @@ trait TreeDSL {
         if (opSym == NoSymbol) ANY_==(other)
         else fn(target, opSym, other)
       }
-      def ANY_EQ  (other: Tree)     = fn(target, nme.eq, toAnyRef(other))
-      def ANY_NE  (other: Tree)     = fn(target, nme.ne, toAnyRef(other))
+      def ANY_EQ  (other: Tree)     = OBJ_EQ(other AS ObjectClass.tpe)
       def ANY_==  (other: Tree)     = fn(target, Any_==, other)
       def ANY_!=  (other: Tree)     = fn(target, Any_!=, other)
       def OBJ_==  (other: Tree)     = fn(target, Object_==, other)
@@ -262,8 +259,8 @@ trait TreeDSL {
         if (target.tpe.typeSymbol == SomeClass) TRUE   // is Some[_]
         else NOT(ID(target) DOT nme.isEmpty)           // is Option[_]
 
-      def IS_NULL() = REF(target) ANY_EQ NULL
-      def NOT_NULL() = REF(target) ANY_NE NULL
+      def IS_NULL()  = REF(target) OBJ_EQ NULL
+      def NOT_NULL() = REF(target) OBJ_NE NULL
 
       def GET() = fn(REF(target), nme.get)
 
diff --git a/src/compiler/scala/tools/nsc/matching/ParallelMatching.scala b/src/compiler/scala/tools/nsc/matching/ParallelMatching.scala
index f22c876085..fb9dffeb66 100644
--- a/src/compiler/scala/tools/nsc/matching/ParallelMatching.scala
+++ b/src/compiler/scala/tools/nsc/matching/ParallelMatching.scala
@@ -881,7 +881,7 @@ trait ParallelMatching extends ast.TreeDSL
 
       typer typed (tpe match {
         case ct: ConstantType => ct.value match {
-            case v @ Constant(null) if scrutTree.tpe.isAnyRef   => scrutTree ANY_EQ NULL
+            case v @ Constant(null) if scrutTree.tpe.isAnyRef   => scrutTree OBJ_EQ NULL
             case v                                              => scrutTree MEMBER_== Literal(v)
           }
         case _: SingletonType if useEqTest                      =>
@@ -916,7 +916,7 @@ trait ParallelMatching extends ast.TreeDSL
 
       outerAccessor(tpe2test.typeSymbol) match {
         case NoSymbol => ifDebug(cunit.warning(scrut.pos, "no outer acc for " + tpe2test.typeSymbol)) ; cond
-        case outerAcc => cond AND (((scrut AS_ANY tpe2test) DOT outerAcc)() ANY_EQ theRef)
+        case outerAcc => cond AND (((scrut AS_ANY tpe2test) DOT outerAcc)() OBJ_EQ theRef)
       }
     }
   }
diff --git a/src/compiler/scala/tools/nsc/transform/CleanUp.scala b/src/compiler/scala/tools/nsc/transform/CleanUp.scala
index 387e7bc6db..db577d04a1 100644
--- a/src/compiler/scala/tools/nsc/transform/CleanUp.scala
+++ b/src/compiler/scala/tools/nsc/transform/CleanUp.scala
@@ -195,7 +195,7 @@ abstract class CleanUp extends Transform with ast.TreeDSL {
               def getMethodSym = ClassClass.tpe member nme.getMethod_
 
               def isCacheEmpty(receiver: Symbol): Tree =
-                reflClassCacheSym.IS_NULL() OR (reflClassCacheSym.GET() ANY_NE REF(receiver))
+                reflClassCacheSym.IS_NULL() OR (reflClassCacheSym.GET() OBJ_NE REF(receiver))
 
               addStaticMethodToClass("reflMethod$Method", List(ClassClass.tpe), MethodClass.tpe) {
                 case Pair(reflMethodSym, List(forReceiverSym)) =>
@@ -245,7 +245,7 @@ abstract class CleanUp extends Transform with ast.TreeDSL {
                   val methodSym = reflMethodSym.newVariable(ad.pos, mkTerm("method")) setInfo MethodClass.tpe
 
                   BLOCK(
-                    IF (getPolyCache ANY_EQ NULL) THEN (REF(reflPolyCacheSym) === mkNewPolyCache) ENDIF,
+                    IF (getPolyCache OBJ_EQ NULL) THEN (REF(reflPolyCacheSym) === mkNewPolyCache) ENDIF,
                     VAL(methodSym) === ((getPolyCache DOT methodCache_find)(REF(forReceiverSym))) ,
                     IF (REF(methodSym) OBJ_!= NULL) .
                       THEN (Return(REF(methodSym)))
diff --git a/src/compiler/scala/tools/nsc/transform/Constructors.scala b/src/compiler/scala/tools/nsc/transform/Constructors.scala
index 34b7b7f95b..c1afe0c3d8 100644
--- a/src/compiler/scala/tools/nsc/transform/Constructors.scala
+++ b/src/compiler/scala/tools/nsc/transform/Constructors.scala
@@ -150,7 +150,7 @@ abstract class Constructors extends Transform with ast.TreeDSL {
           result =
             atPos(to.pos) {
               localTyper.typed {
-                IF (from ANY_EQ NULL) THEN THROW(NullPointerExceptionClass) ELSE result
+                IF (from OBJ_EQ NULL) THEN THROW(NullPointerExceptionClass) ELSE result
               }
             }
 
diff --git a/src/compiler/scala/tools/nsc/transform/UnCurry.scala b/src/compiler/scala/tools/nsc/transform/UnCurry.scala
index 45e7410bc5..c4074d7efc 100644
--- a/src/compiler/scala/tools/nsc/transform/UnCurry.scala
+++ b/src/compiler/scala/tools/nsc/transform/UnCurry.scala
@@ -630,7 +630,10 @@ abstract class UnCurry extends InfoTransform with TypingTransformers {
           }
           tree1
       }
-    } setType uncurryTreeType(tree.tpe)
+    } setType {
+      assert(tree.tpe != null, tree + " tpe is null")
+      uncurryTreeType(tree.tpe)
+    }
 
     def postTransform(tree: Tree): Tree = atPhase(phase.next) {
       def applyUnary(): Tree = {
diff --git a/src/compiler/scala/tools/nsc/typechecker/Infer.scala b/src/compiler/scala/tools/nsc/typechecker/Infer.scala
index 16de39b977..f8466aed33 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Infer.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Infer.scala
@@ -246,10 +246,13 @@ trait Infer {
       if (sym.isError) {
         tree setSymbol sym setType ErrorType
       } else {
-        def accessError(explanation: String): Tree =
-          errorTree(tree, underlying(sym).toString() + " cannot be accessed in " +
+        def accessError(explanation: String): Tree = {
+          val realsym = underlying(sym)
+
+          errorTree(tree, realsym + realsym.locationString + " cannot be accessed in " +
                     (if (sym.isClassConstructor) context.enclClass.owner else pre.widen) +
                     explanation)
+        }
 
         val topClass = context.owner.toplevelClass
         if (context.unit != null)
diff --git a/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala b/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala
index 549e4d325f..b28b92357f 100644
--- a/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala
@@ -142,14 +142,12 @@ trait SyntheticMethods extends ast.TreeDSL {
     /** The equality method for case modules:
      *   def equals(that: Any) = this eq that
      */
-    def equalsModuleMethod: Tree = localTyper typed {
+    def equalsModuleMethod: Tree = {
       val method = makeEqualityMethod(nme.equals_)
       val that   = method ARG 0
 
       localTyper typed {
-        DEF(method) === {
-          (This(clazz) DOT Object_eq)(that AS AnyRefClass.tpe)
-        }
+        DEF(method) === (This(clazz) ANY_EQ that)
       }
     }
 
@@ -197,10 +195,12 @@ trait SyntheticMethods extends ast.TreeDSL {
 
       // Verify with canEqual method before returning true.
       def canEqualCheck() = {
-        val that: Tree              = typer typed ((method ARG 0) AS clazz.tpe)
+        val that: Tree              = (method ARG 0) AS clazz.tpe
         val canEqualOther: Symbol   = clazz.info nonPrivateMember nme.canEqual_
 
-        (that DOT canEqualOther)(This(clazz))
+        typer typed {
+          (that DOT canEqualOther)(This(clazz))
+        }
       }
 
       // Pattern is classname applied to parameters, and guards are all logical and-ed
@@ -228,12 +228,12 @@ trait SyntheticMethods extends ast.TreeDSL {
         result
     }
 
-    def needsReadResolve =
+    def needsReadResolve = (
       // only nested objects inside objects should get readResolve automatically
       // otherwise after de-serialization we get null references for lazy accessors (nested object -> lazy val + class def)
       clazz.isSerializable &&
       ((!clazz.owner.isPackageClass && clazz.owner.isModuleClass) || clazz.owner.isPackageClass)
-       //(clazz.companionClass != NoSymbol))
+    )
 
     // A buffer collecting additional methods for the template body
     val ts = new ListBuffer[Tree]
@@ -323,6 +323,11 @@ trait SyntheticMethods extends ast.TreeDSL {
         if (!reporter.hasErrors) throw ex
     }
 
-    treeCopy.Template(templ, templ.parents, templ.self, templ.body ++ ts.toList)
+    if (phase.id <= currentRun.typerPhase.id) {
+      treeCopy.Template(templ, templ.parents, templ.self,
+        if (ts.isEmpty) templ.body else templ.body ++ ts // avoid copying templ.body if empty
+      )
+    }
+    else templ
   }
 }
diff --git a/src/compiler/scala/tools/nsc/typechecker/TreeCheckers.scala b/src/compiler/scala/tools/nsc/typechecker/TreeCheckers.scala
index de9da9d814..2f271f237f 100644
--- a/src/compiler/scala/tools/nsc/typechecker/TreeCheckers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/TreeCheckers.scala
@@ -7,17 +7,36 @@ package scala.tools.nsc
 package typechecker
 
 import scala.tools.nsc.symtab.Flags._
+import scala.collection.mutable
+import mutable.HashMap
+import util.returning
 
 abstract class TreeCheckers extends Analyzer {
-
   import global._
 
-  val tpeOfTree = new scala.collection.mutable.HashMap[Tree, Type]
+  lazy val tpeOfTree = new HashMap[Tree, Type]
+
+  def posstr(p: Position) =
+    try p.source.path + ":" + p.line
+    catch { case _: UnsupportedOperationException => p.toString }
+
+  def errorFn(pos: Position, msg: Any) = println("[%s] %s: %s".format(phase, posstr(pos), msg))
+  def assertFn(cond: Boolean, msg: => Any) =
+    if (!cond) errorFn(NoPosition, msg)
 
   def checkTrees {
     if (settings.verbose.value)
       Console.println("[consistency check at the beginning of phase " + phase + "]")
-    for (unit <- currentRun.units) check(unit)
+
+    currentRun.units foreach check
+  }
+
+  def printingTypings[T](body: => T): T = {
+    val saved = global.printTypings
+    global.printTypings = true
+    val result = body
+    global.printTypings = saved
+    result
   }
 
   def check(unit: CompilationUnit) {
@@ -30,96 +49,121 @@ abstract class TreeCheckers extends Analyzer {
     val unit0 = currentRun.currentUnit
     currentRun.currentUnit = unit
     checker.precheck.traverse(unit.body)
+    // printingTypings(checker.typed(unit.body))
     checker.typed(unit.body)
     checker.postcheck.traverse(unit.body)
     currentRun.advanceUnit
-    assert(currentRun.currentUnit == unit)
+    assertFn(currentRun.currentUnit == unit, "currentRun.currentUnit == unit")
     currentRun.currentUnit = unit0
   }
 
   override def newTyper(context: Context): Typer = new TreeChecker(context)
 
   class TreeChecker(context0: Context) extends Typer(context0) {
-    import infer._
-
-    override def typed(tree: Tree, mode: Int, pt: Type): Tree = {
-      tree match {
-        case EmptyTree | TypeTree() =>
-          ;
-        case _ =>
-          if (!tpeOfTree.contains(tree)) {
-            tpeOfTree.update(tree, tree.tpe)
-            tree.tpe = null
-          }
-          val newtree = super.typed(tree, mode, pt);
-          if ((newtree ne tree) && !newtree.isInstanceOf[Literal])
-            error(tree.pos, "trees differ\n old: " + tree + " [" + tree.getClass() +
-                  "]\n new: " + newtree + " [" + newtree.getClass() + "]")
-      }
-      tree
+    private def treestr(t: Tree)    = t + " [" + t.getClass() + "]"
+    private def ownerstr(s: Symbol) = "" + s + s.locationString
+
+    private def treesDiffer(t1: Tree, t2: Tree) =
+      errorFn(t1.pos, "trees differ\n old: " + treestr(t1) + "\n new: " + treestr(t2))
+    private def typesDiffer(tree: Tree, tp1: Type, tp2: Type) =
+      errorFn(tree.pos, "types differ\n old: " + tp1 + "\n new: " + tp2 + "\n tree: " + tree)
+    private def ownersDiffer(tree: Tree, shouldBe: Symbol) = {
+      val sym = tree.symbol
+      errorFn(tree.pos, sym + " has wrong owner: " + ownerstr(sym.owner) + ", should be: " + ownerstr(shouldBe))
+    }
+
+    override def typed(tree: Tree, mode: Int, pt: Type): Tree = returning(tree) {
+      case EmptyTree | TypeTree() => ()
+      case _ if tree.tpe != null  =>
+        tpeOfTree.getOrElseUpdate(tree, {
+          val saved = tree.tpe
+          tree.tpe = null
+          saved
+        })
+        super.typed(tree, mode, pt) match {
+          case _: Literal     => ()
+          case x if x ne tree => treesDiffer(tree, x)
+          case _              => ()
+        }
+      case _ => ()
     }
 
     object precheck extends Traverser {
       override def traverse(tree: Tree) {
+        val sym = tree.symbol
+        def accessed = sym.accessed
+        def fail(msg: String) = errorFn(tree.pos, msg + tree.getClass + " / " + tree)
+
         tree match {
           case DefDef(_, _, _, _, _, _) =>
-            if (tree.symbol.hasFlag(ACCESSOR) &&
-                !tree.symbol.isDeferred &&
-                !tree.symbol.tpe.resultType.isInstanceOf[ConstantType]) {
-              assert(tree.symbol.accessed != NoSymbol, tree.symbol)
-              assert(tree.symbol.accessed.getter(tree.symbol.owner) == tree.symbol ||
-                     tree.symbol.accessed.setter(tree.symbol.owner) == tree.symbol)
+            if (sym.isGetterOrSetter && !sym.isDeferred) {
+              sym.tpe.resultType match {
+                case _: ConstantType  => ()
+                case _                =>
+                  assertFn(accessed != NoSymbol, sym)
+                  assertFn(
+                    accessed.getter(sym.owner) == sym || accessed.setter(sym.owner) == sym,
+                    "accessed.getter(sym.owner) == sym || accessed.setter(sym.owner) == sym"
+                  )
+              }
             }
           case ValDef(_, _, _, _) =>
-            if (tree.symbol.hasGetter) {
-              assert(tree.symbol.getter(tree.symbol.owner) != NoSymbol, tree.symbol)
+            if (sym.hasGetter) {
+              assertFn(sym.getter(sym.owner) != NoSymbol, sym)
             }
           case Apply(_, args) =>
-            assert(args forall (EmptyTree !=))
-          case Select(_, _) =>
-            assert(tree.symbol != NoSymbol, tree)
+            assertFn(args forall (_ != EmptyTree), args)
+          case Select(qual, name) =>
+            if (sym == NoSymbol)
+              errorFn(tree.pos, "NoSymbol: " + tree)
           case This(_) =>
-            if (!(tree.symbol.isStatic && (tree.symbol hasFlag MODULE))) {
-              if (currentOwner.ownerChain takeWhile (_ != tree.symbol) exists (_ == NoSymbol)) {
-                error(tree.pos, "tree symbol "+tree.symbol+" does not point to enclosing class; tree = "+tree)
-                return
-              }
-            }
+            if (sym == NoSymbol) errorFn(tree.pos, "NoSymbol: " + tree)
+            else if (sym.isStatic && (sym hasFlag MODULE)) ()
+            else if (currentOwner.ownerChain takeWhile (_ != sym) exists (_ == NoSymbol))
+              return fail("tree symbol "+sym+" does not point to enclosing class; tree = ")
+
+          /** Temporary while Import nodes are untyped. */
+          case Import(_, _) =>
+            return
           case _ =>
         }
-        if (tree.pos == NoPosition && tree != EmptyTree) {
-          error(tree.pos, "tree without position: " + tree)
-        } else if ((tree.tpe eq null) && phase.id >= currentRun.typerPhase.id) {
-          error(tree.pos, "tree without type: " + tree)
-        } else if (tree.isDef && tree.symbol.owner != currentOwner) {
-          var owner = currentOwner
-          while (owner.isTerm && !owner.isMethod && tree.symbol.owner != owner)
-            owner = owner.owner;
-          if (tree.symbol.owner != owner) {
-            error(tree.pos, "" + tree.symbol + " has wrong owner: " + tree.symbol.owner +
-                  tree.symbol.owner.locationString + ", should be: " +
-                  currentOwner + currentOwner.locationString)
+
+        if (tree.pos == NoPosition && tree != EmptyTree)
+          fail("tree without position: ")
+        else if (tree.tpe == null && phase.id > currentRun.typerPhase.id)
+          fail("tree without type: ")
+        else if (tree.isDef) {
+          if (sym == NoSymbol)
+            fail("DefTree with NoSymbol: ")
+          else tree match {
+            case x: PackageDef    =>
+              if (sym.ownerChain contains currentOwner) ()
+              else fail(sym + " owner chain does not contain currentOwner " + currentOwner)
+            case _ =>
+              def cond(s: Symbol) = s.isTerm && !s.isMethod && s != sym.owner
+
+              if (sym.owner != currentOwner) {
+                val found = currentOwner.ownerChain find (x => !cond(x)) getOrElse fail("DefTree can't find owner: ")
+                if (sym.owner != found)
+                  fail("Expected owner %s, found %s: ".format(found, sym.owner))
+              }
           }
-        } else {
-          super.traverse(tree)
         }
+        super.traverse(tree)
       }
     }
 
     object postcheck extends Traverser {
       override def traverse(tree: Tree) {
         tree match {
-          case EmptyTree | TypeTree() =>
-            ;
+          case EmptyTree | TypeTree() => ()
           case _ =>
-            tpeOfTree.get(tree) match {
-              case Some(oldtpe) =>
-                if (!(oldtpe =:= tree.tpe))
-                  error(tree.pos, "types differ\n old: " + oldtpe +
-                        "\n new: " + tree.tpe + "\n tree: " + tree)
-                tree.tpe = oldtpe
-                super.traverse(tree)
-              case None =>
+            tpeOfTree get tree foreach { oldtpe =>
+              if (oldtpe =:= tree.tpe) ()
+              else typesDiffer(tree, oldtpe, tree.tpe)
+
+              tree.tpe = oldtpe
+              super.traverse(tree)
             }
         }
       }