Did a bunch of symbol oriented work on checkers.

changes in Global and Typer to accomodate this, and renamed "Checkers"
to "ICodeCheckers" to make some less confusing space for the future
"SymbolCheckers". No review.

6 files changed, 146 insertions, 27 deletions

diff --git a/src/compiler/scala/tools/nsc/Global.scala b/src/compiler/scala/tools/nsc/Global.scala
index ff3a5cdc0b..d3f505d0e2 100644
--- a/src/compiler/scala/tools/nsc/Global.scala
+++ b/src/compiler/scala/tools/nsc/Global.scala
@@ -24,7 +24,7 @@ import ast.parser._
 import typechecker._
 import transform._
 
-import backend.icode.{ ICodes, GenICode, Checkers }
+import backend.icode.{ ICodes, GenICode, ICodeCheckers }
 import backend.{ ScalaPrimitives, Platform, MSILPlatform, JavaPlatform }
 import backend.jvm.GenJVM
 import backend.opt.{ Inliners, ClosureElimination, DeadCodeElimination }
@@ -78,11 +78,6 @@ class Global(var settings: Settings, var reporter: Reporter) extends SymbolTable
     val global: Global.this.type = Global.this
   } with ConstantFolder
 
-  /** Tree checker (used for testing and debugging) */
-  object checker extends {
-    val global: Global.this.type = Global.this
-  } with TreeCheckers
-
   /** ICode generator */
   object icodes extends {
     val global: Global.this.type = Global.this
@@ -98,11 +93,6 @@ class Global(var settings: Settings, var reporter: Reporter) extends SymbolTable
     val global: Global.this.type = Global.this
   } with CopyPropagation
 
-  /** Icode verification */
-  object checkers extends {
-    val global: Global.this.type = Global.this
-  } with Checkers
-
   /** Some statistics (normally disabled) */
   object statistics extends {
     val global: Global.this.type = Global.this
@@ -515,7 +505,21 @@ class Global(var settings: Settings, var reporter: Reporter) extends SymbolTable
     val runsRightAfter = None
   } with SampleTransform
 
-  object icodeChecker extends checkers.ICodeChecker()
+  /** The checkers are for validating the compiler data structures
+   *  at phase boundaries.
+   */
+
+  /** Tree checker */
+  object treeChecker extends {
+    val global: Global.this.type = Global.this
+  } with TreeCheckers
+
+  /** Icode verification */
+  object icodeCheckers extends {
+    val global: Global.this.type = Global.this
+  } with ICodeCheckers
+
+  object icodeChecker extends icodeCheckers.ICodeChecker()
 
   object typer extends analyzer.Typer(
     analyzer.NoContext.make(EmptyTree, Global.this.definitions.RootClass, new Scope))
@@ -777,7 +781,7 @@ class Global(var settings: Settings, var reporter: Reporter) extends SymbolTable
             phase = globalPhase
             inform("[Now checking: " + phase.prev.name + "]")
             if (globalPhase.id >= icodePhase.id) icodeChecker.checkICodes
-            else checker.checkTrees
+            else treeChecker.checkTrees
           }
           else inform("[Not checkable: " + globalPhase.prev.name + "]")
         }
diff --git a/src/compiler/scala/tools/nsc/backend/icode/Checkers.scala b/src/compiler/scala/tools/nsc/backend/icode/ICodeCheckers.scala
index 61f2d8dd25..53205e26ca 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/Checkers.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/ICodeCheckers.scala
@@ -10,7 +10,7 @@ package icode
 import scala.collection.mutable.{Buffer, ListBuffer, Map, HashMap}
 import scala.tools.nsc.symtab._
 
-abstract class Checkers {
+abstract class ICodeCheckers {
   val global: Global
   import global._
 
@@ -108,11 +108,11 @@ abstract class Checkers {
 
       for (f1 <- cls.fields ; f2 <- cls.fields ; if f1 < f2)
         if (isConfict(f1, f2, false))
-          Checkers.this.global.error("Repetitive field name: " + f1.symbol.fullName)
+          ICodeCheckers.this.global.error("Repetitive field name: " + f1.symbol.fullName)
 
       for (m1 <- cls.methods ; m2 <- cls.methods ; if m1 < m2)
         if (isConfict(m1, m2, true))
-          Checkers.this.global.error("Repetitive method: " + m1.symbol.fullName)
+          ICodeCheckers.this.global.error("Repetitive method: " + m1.symbol.fullName)
 
       clasz.methods foreach check
     }
@@ -633,7 +633,7 @@ abstract class Checkers {
     //////////////// Error reporting /////////////////////////
 
     def error(msg: String) {
-      Checkers.this.global.error("!! ICode checker fatality in " + method + " at:" + blockAsString(basicBlock) + ":\n " + msg)
+      ICodeCheckers.this.global.error("!! ICode checker fatality in " + method + " at:" + blockAsString(basicBlock) + ":\n " + msg)
     }
 
     def error(msg: String, stack: TypeStack) {
diff --git a/src/compiler/scala/tools/nsc/symtab/Types.scala b/src/compiler/scala/tools/nsc/symtab/Types.scala
index 21da19c526..9dac3c530b 100644
--- a/src/compiler/scala/tools/nsc/symtab/Types.scala
+++ b/src/compiler/scala/tools/nsc/symtab/Types.scala
@@ -814,9 +814,19 @@ trait Types extends reflect.generic.Types { self: SymbolTable =>
       // of a subtyping/equality judgement, which can lead to recursive types being constructed.
       // See (t0851) for a situation where this happens.
       if (!this.isGround) {
+        // PP: The foreach below was formerly expressed as:
+        //   for(tv @ TypeVar(_, _) <- this) { suspension suspend tv }
+        //
+        // The tree checker failed this saying a TypeVar is required, but a (Type @unchecked) was found.
+        // This is a consequence of using a pattern match and variable binding + ticket #1503, which
+        // was addressed by weakening the type of bindings in pattern matches if they occur on the right.
+        // So I'm not quite sure why this works at all, as the checker is right that it is mistyped.
+        // For now I modified it as below, which achieves the same without error.
+        //
         // make each type var in this type use its original type for comparisons instead of collecting constraints
-        for(tv@TypeVar(_, _) <- this) {
-          suspension suspend tv
+        this foreach {
+          case tv: TypeVar  => suspension suspend tv
+          case _            => ()
         }
       }
 
diff --git a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala
index 1403377367..0e8faa1f1e 100644
--- a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala
+++ b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala
@@ -9,6 +9,7 @@ package transform
 import symtab._
 import Flags.{ CASE => _, _ }
 import scala.collection.mutable.ListBuffer
+import scala.collection.mutable
 import matching.{ Patterns, ParallelMatching }
 
 /** This class ...
diff --git a/src/compiler/scala/tools/nsc/typechecker/TreeCheckers.scala b/src/compiler/scala/tools/nsc/typechecker/TreeCheckers.scala
index 2bc3103854..16961db77b 100644
--- a/src/compiler/scala/tools/nsc/typechecker/TreeCheckers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/TreeCheckers.scala
@@ -8,12 +8,108 @@ package typechecker
 
 import scala.tools.nsc.symtab.Flags._
 import scala.collection.mutable
-import mutable.HashMap
+import mutable.{ HashMap, HashSet, ListBuffer }
 import util.returning
 
 abstract class TreeCheckers extends Analyzer {
   import global._
 
+  private def classstr(x: AnyRef) = x.getClass.getName split """\\.|\\$""" last;
+  private def typestr(x: Type)    = " (tpe = " + x + ")"
+  private def treestr(t: Tree)    = t + " [" + classstr(t) + "]" + typestr(t.tpe)
+  private def ownerstr(s: Symbol) = "'" + s + "'" + s.locationString
+  private def wholetreestr(t: Tree) = nodeToString(t) + "\n"
+
+  private def beststr(t: Tree) = "<" + {
+    if (t.symbol != null && t.symbol != NoSymbol) "sym=" + ownerstr(t.symbol)
+    else if (t.tpe.isComplete) "tpe=" + typestr(t.tpe)
+    else t match {
+      case x: DefTree => "name=" + x.name
+      case x: RefTree => "reference=" + x.name
+      case _          => "clazz=" + classstr(t)
+    }
+  } + ">"
+
+  /** This is a work in progress, don't take it too seriously.
+   */
+  object SymbolTracker extends Traverser {
+    type PhaseMap = HashMap[Symbol, List[Tree]]
+    val maps: ListBuffer[(Phase, PhaseMap)] = ListBuffer()
+    def prev          = maps.init.last._2
+    def latest        = maps.last._2
+    val defSyms       = new HashMap[Symbol, List[DefTree]]
+    val newSyms       = new HashSet[Symbol]
+    val movedMsgs     = new ListBuffer[String]
+    def sortedNewSyms = newSyms.toList.distinct sortBy (_.name.toString)
+
+    def inPrev(sym: Symbol) = {
+      (maps.size >= 2) && (prev contains sym)
+    }
+    def record(sym: Symbol, tree: Tree) = {
+      if (latest contains sym) latest(sym) = latest(sym) :+ tree
+      else latest(sym) = List(tree)
+
+      if (inPrev(sym)) {
+        val prevTrees = prev(sym)
+
+        if (prevTrees exists (t => (t eq tree) || (t.symbol == sym))) ()
+        else if (prevTrees exists (_.symbol.owner == sym.owner.implClass)) {
+          errorFn("Noticed " + ownerstr(sym) + " moving to implementation class.")
+        }
+        else {
+          val s1 = (prevTrees map wholetreestr).sorted.distinct
+          val s2 = wholetreestr(tree)
+          if (s1 contains s2) ()
+          else movedMsgs += ("\n** %s moved:\n** Previously:\n%s\n** Currently:\n%s".format(ownerstr(sym), s1 mkString ", ", s2))
+        }
+      }
+      else newSyms += sym
+    }
+    def reportChanges(): Unit = {
+      // new symbols
+      if (newSyms.nonEmpty) {
+        val str =
+          if (settings.debug.value) "New symbols: " + (sortedNewSyms mkString " ")
+          else newSyms.size + " new symbols."
+
+        newSyms.clear()
+        errorFn(str)
+      }
+
+      // moved symbols
+      movedMsgs foreach errorFn
+      movedMsgs.clear()
+
+      // duplicate defs
+      for ((sym, defs) <- defSyms ; if defs.size > 1) {
+        errorFn("%s DefTrees with symbol '%s': %s".format(defs.size, ownerstr(sym), defs map beststr mkString ", "))
+      }
+      defSyms.clear()
+    }
+
+    def check(ph: Phase, unit: CompilationUnit): Unit = {
+      if (maps.isEmpty || maps.last._1 != ph)
+        maps += ((ph, new PhaseMap))
+
+      traverse(unit.body)
+      reportChanges()
+    }
+    override def traverse(tree: Tree): Unit = {
+      val sym    = tree.symbol
+      if (sym != null && sym != NoSymbol) {
+        record(sym, tree)
+        tree match {
+          case x: DefTree =>
+            if (defSyms contains sym) defSyms(sym) = defSyms(sym) :+ x
+            else defSyms(sym) = List(x)
+          case _ => ()
+        }
+      }
+
+      super.traverse(tree)
+    }
+  }
+
   lazy val tpeOfTree = new HashMap[Tree, Type]
 
   def posstr(p: Position) =
@@ -48,12 +144,12 @@ abstract class TreeCheckers extends Analyzer {
     assertFn(currentRun.currentUnit == unit, "currentUnit is " + currentRun.currentUnit + ", but unit is " + unit)
     currentRun.currentUnit = unit0
   }
-
   def check(unit: CompilationUnit) {
     informProgress("checking "+unit)
     val context = rootContext(unit)
     context.checking = true
     tpeOfTree.clear
+    SymbolTracker.check(phase, unit)
     val checker = new TreeChecker(context)
     runWithUnit(unit) {
       checker.precheck.traverse(unit.body)
@@ -65,10 +161,11 @@ abstract class TreeCheckers extends Analyzer {
   override def newTyper(context: Context): Typer = new TreeChecker(context)
 
   class TreeChecker(context0: Context) extends Typer(context0) {
-    private def classstr(x: AnyRef) = x.getClass.getName split '.' last;
-    private def typestr(x: Type)    = " (tpe = " + x + ")"
-    private def treestr(t: Tree)    = t + " [" + classstr(t) + "]" + typestr(t.tpe)
-    private def ownerstr(s: Symbol) = "'" + s + "'" + s.locationString
+    override protected def typerAddSyntheticMethods(templ: Template, clazz: Symbol, context: Context): Template = {
+      // If we don't intercept this all the synthetics get added at every phase,
+      // with predictably unfortunate results.
+      templ
+    }
 
     // XXX check for tree.original on TypeTrees.
     private def treesDiffer(t1: Tree, t2: Tree) =
diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
index 6a67fecd80..647e5e422d 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -1301,7 +1301,7 @@ trait Typers { self: Analyzer =>
       val tparams1 = cdef.tparams mapConserve (typedTypeDef)
       val impl1 = newTyper(context.make(cdef.impl, clazz, new Scope))
         .typedTemplate(cdef.impl, parentTypes(cdef.impl))
-      val impl2 = addSyntheticMethods(impl1, clazz, context)
+      val impl2 = typerAddSyntheticMethods(impl1, clazz, context)
       if ((clazz != ClassfileAnnotationClass) &&
           (clazz isNonBottomSubClass ClassfileAnnotationClass))
         unit.warning (cdef.pos,
@@ -1338,7 +1338,7 @@ trait Typers { self: Analyzer =>
       assert(clazz != NoSymbol)
       val impl1 = newTyper(context.make(mdef.impl, clazz, new Scope))
         .typedTemplate(mdef.impl, parentTypes(mdef.impl))
-      val impl2 = addSyntheticMethods(impl1, clazz, context)
+      val impl2 = typerAddSyntheticMethods(impl1, clazz, context)
 
       if (mdef.name == nme.PACKAGEkw)
         for (m <- mdef.symbol.info.members)
@@ -1348,6 +1348,13 @@ trait Typers { self: Analyzer =>
       treeCopy.ModuleDef(mdef, typedMods, mdef.name, impl2) setType NoType
     }
 
+    /** In order to override this in the TreeCheckers Typer so synthetics aren't re-added
+     *  all the time, it is exposed here the module/class typing methods go through it.
+     */
+    protected def typerAddSyntheticMethods(templ: Template, clazz: Symbol, context: Context): Template = {
+      addSyntheticMethods(templ, clazz, context)
+    }
+
     /**
      *  @param stat ...
      *  @return     ...