diff options
author | Paul Phillips <paulp@improving.org> | 2013-09-09 12:25:26 -0700 |
---|---|---|
committer | Paul Phillips <paulp@improving.org> | 2013-09-09 12:53:32 -0700 |
commit | 288b12e48af3222134f3985b65e40f202af81df2 (patch) | |
tree | 87dedc0c05be446bf8ad76f1aa760af13ed145bb /src/compiler | |
parent | f342445f68654c1606871b5d7372621558731f65 (diff) | |
download | scala-288b12e48af3222134f3985b65e40f202af81df2.tar.gz scala-288b12e48af3222134f3985b65e40f202af81df2.tar.bz2 scala-288b12e48af3222134f3985b65e40f202af81df2.zip |
Noise reduction + minor enhance in TreeCheckers.
Misc irrelevant work, which I can only offer as-is. It lowers
the noise in -Ycheck:* output and performs some common sense
chillaxes like not screaming ERROR IN INTERNAL CHECKING! WE'RE
ALL GOING TO DIE! when a tree doesn't hit all nine points at
the Jiffy Tree.
You can see some reasonably well reduced symbol flailing if
you run the included pending tests:
test/partest --show-diff test/pending/pos/treecheckers
Example output,
Out of scope symbol reference {
tree TypeTree Factory[Traversable]
position OffsetPosition test/pending/pos/treecheckers/c5.scala:3
with sym ClassSymbol Factory: Factory[CC]
and tpe ClassArgsTypeRef Factory[Traversable]
encl(1) ModuleSymbol object Test5
ref to AbstractTypeSymbol X (<deferred> <param>)
}
Diffstat (limited to 'src/compiler')
-rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/Contexts.scala | 6 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/TreeCheckers.scala | 351 |
2 files changed, 240 insertions, 117 deletions
diff --git a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala index 8d42bf94f3..cd2b9b3a97 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala @@ -19,6 +19,8 @@ trait Contexts { self: Analyzer => import definitions.{ JavaLangPackage, ScalaPackage, PredefModule, ScalaXmlTopScope, ScalaXmlPackage } import ContextMode._ + protected def onTreeCheckerError(pos: Position, msg: String): Unit = () + object NoContext extends Context(EmptyTree, NoSymbol, EmptyScope, NoCompilationUnit, null) { // We can't pass the uninitialized `this`. Instead, we treat null specially in `Context#outer` @@ -531,8 +533,8 @@ trait Contexts { self: Analyzer => if (msg endsWith ds) msg else msg + ds } - private def unitError(pos: Position, msg: String) = - unit.error(pos, if (checking) "\n**** ERROR DURING INTERNAL CHECKING ****\n" + msg else msg) + private def unitError(pos: Position, msg: String): Unit = + if (checking) onTreeCheckerError(pos, msg) else unit.error(pos, msg) @inline private def issueCommon(err: AbsTypeError)(pf: PartialFunction[AbsTypeError, Unit]) { if (settings.Yissuedebug) { diff --git a/src/compiler/scala/tools/nsc/typechecker/TreeCheckers.scala b/src/compiler/scala/tools/nsc/typechecker/TreeCheckers.scala index 1c8d37ef39..c354f8707b 100644 --- a/src/compiler/scala/tools/nsc/typechecker/TreeCheckers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/TreeCheckers.scala @@ -9,15 +9,69 @@ package typechecker import scala.collection.mutable import mutable.ListBuffer import util.returning +import scala.reflect.internal.util.shortClassOfInstance +import scala.reflect.internal.util.StringOps._ abstract class TreeCheckers extends Analyzer { import global._ - private def classstr(x: AnyRef) = (x.getClass.getName split """\\.|\\$""").last + override protected def onTreeCheckerError(pos: Position, msg: String) { + if (settings.fatalWarnings) + currentUnit.warning(pos, "\n** Error during internal checking:\n" + msg) + } + + case class DiffResult[T](lost: List[T], gained: List[T]) { + def isEmpty = lost.isEmpty && gained.isEmpty + def lost_s = if (lost.isEmpty) "" else lost.mkString("lost: ", ", ", "") + def gained_s = if (gained.isEmpty) "" else gained.mkString("gained: ", ", ", "") + override def toString = ojoin(lost_s, gained_s) + } + + def diffList[T](xs: List[T], ys: List[T]): DiffResult[T] = + DiffResult(xs filterNot ys.contains, ys filterNot xs.contains) + + def diffTrees(t1: Tree, t2: Tree): DiffResult[Tree] = + diffList(t1 filter (_ ne t1), t2 filter (_ ne t2)) + + def diffTemplates(t1: Template, t2: Template): String = { + val parents = diffList(t1.parents, t2.parents).toString match { case "" => "" case s => "parents " + s } + val stats = diffList(t1.body, t2.body).toString match { case "" => "" case s => "stats " + s } + oempty(parents, stats) mkString ", " + } + + def diff(t1: Tree, t2: Tree): String = (t1, t2) match { + case (_: Literal, _: Literal) => "" + case (t1: ImplDef, t2: ImplDef) => diff(t1.impl, t2.impl) + case (t1: Template, t2: Template) => diffTemplates(t1, t2) + case _ => diffTrees(t1, t2).toString // "<error: different tree classes>" + } + + private def clean_s(s: String) = s.replaceAllLiterally("scala.collection.", "s.c.") private def typestr(x: Type) = " (tpe = " + x + ")" - private def treestr(t: Tree) = t + " [" + classstr(t) + "]" + typestr(t.tpe) + private def treestr(t: Tree) = t + " [" + classString(t) + "]" + typestr(t.tpe) private def ownerstr(s: Symbol) = "'" + s + "'" + s.locationString private def wholetreestr(t: Tree) = nodeToString(t) + "\n" + private def truncate(str: String, len: Int): String = ( + if (str.length <= len) str + else (str takeWhile (_ != '\n') take len - 3) + "..." + ) + private def signature(sym: Symbol) = clean_s(sym match { + case null => "null" + case _: ClassSymbol => sym.name + ": " + sym.tpe_* + case _ => sym.defString + }) + private def classString(x: Any) = x match { + case null => "" + case t: Tree => t.shortClass + case s: Symbol => s.shortSymbolClass + case x: AnyRef => shortClassOfInstance(x) + } + private def nonPackageOwners(s: Symbol) = s.ownerChain drop 1 takeWhile (!_.hasPackageFlag) + private def nonPackageOwnersPlusOne(s: Symbol) = nonPackageOwners(s) ::: (s.ownerChain dropWhile (!_.hasPackageFlag) take 1) + private def ownersString(s: Symbol) = nonPackageOwnersPlusOne(s) match { + case Nil => "NoSymbol" + case xs => xs mkString " -> " + } private def beststr(t: Tree) = "<" + { if (t.symbol != null && t.symbol != NoSymbol) "sym=" + ownerstr(t.symbol) @@ -25,46 +79,50 @@ abstract class TreeCheckers extends Analyzer { else t match { case x: DefTree => "name=" + x.name case x: RefTree => "reference=" + x.name - case _ => "clazz=" + classstr(t) + case _ => "clazz=" + classString(t) } } + ">" /** This is a work in progress, don't take it too seriously. */ object SymbolTracker extends Traverser { - type PhaseMap = mutable.HashMap[Symbol, List[Tree]] + type PhaseMap = mutable.Map[Symbol, List[Tree]] + def symbolTreeMap[T <: Tree]() = mutable.Map[Symbol, List[T]]() withDefaultValue Nil - val maps = ListBuffer[(Phase, PhaseMap)]() - def prev = maps.init.last._2 - def latest = maps.last._2 - val defSyms = mutable.HashMap[Symbol, List[DefTree]]() + var maps: List[(Phase, PhaseMap)] = ((NoPhase, null)) :: Nil + def prev = maps.tail.head._2 + def latest = maps.head._2 + val defSyms = symbolTreeMap[DefTree]() val newSyms = mutable.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) + def record(tree: Tree) { + val sym = tree.symbol + if ((sym eq null) || (sym eq NoSymbol)) return - if (inPrev(sym)) { - val prevTrees = prev(sym) + val prevMap = maps.tail.head._2 + val prevTrees = if (prevMap eq null) Nil else prevMap(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)) - } + tree match { + case t: DefTree => defSyms(sym) ::= t + case _ => + } + + if (prevTrees.isEmpty) + newSyms += sym + else 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) { @@ -88,37 +146,34 @@ abstract class TreeCheckers extends Analyzer { } def check(ph: Phase, unit: CompilationUnit): Unit = { - if (maps.isEmpty || maps.last._1 != ph) - maps += ((ph, new PhaseMap)) - + maps match { + case ((`ph`, _)) :: _ => + case _ => maps ::= ((ph, symbolTreeMap[Tree]())) + } 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 _ => () - } - } - + override def traverse(tree: Tree) { + record(tree) super.traverse(tree) } } lazy val tpeOfTree = mutable.HashMap[Tree, Type]() + private lazy val reportedAlready = mutable.HashSet[(Tree, Symbol)]() + + def posstr(t: Tree): String = if (t eq null) "" else posstr(t.pos) + def posstr(p: Position): String = ( + if (p eq null) "" else { + try p.source.path + ":" + p.line + catch { case _: UnsupportedOperationException => p.toString } + } + ) - def posstr(p: Position) = - try p.source.path + ":" + p.line - catch { case _: UnsupportedOperationException => p.toString } - private var hasError: Boolean = false - def errorFn(msg: Any): Unit = {hasError = true; println("[check: %s] %s".format(phase.prev, msg))} + def errorFn(msg: Any): Unit = Console.err println "[check: %s] %s".format(phase.prev, msg) def errorFn(pos: Position, msg: Any): Unit = errorFn(posstr(pos) + ": " + msg) + def informFn(msg: Any) { if (settings.verbose || settings.debug) println("[check: %s] %s".format(phase.prev, msg)) @@ -127,12 +182,13 @@ abstract class TreeCheckers extends Analyzer { def assertFn(cond: Boolean, msg: => Any) = if (!cond) errorFn(msg) - private def wrap[T](msg: => Any)(body: => Unit) { + private def wrap[T](msg: => Any)(body: => T): T = { try body catch { case x: Throwable => Console.println("Caught " + x) Console.println(msg) x.printStackTrace + null.asInstanceOf[T] } } @@ -144,7 +200,6 @@ abstract class TreeCheckers extends Analyzer { } def runWithUnit[T](unit: CompilationUnit)(body: => Unit): Unit = { - hasError = false val unit0 = currentUnit currentRun.currentUnit = unit body @@ -163,22 +218,28 @@ abstract class TreeCheckers extends Analyzer { checker.precheck.traverse(unit.body) checker.typed(unit.body) checker.postcheck.traverse(unit.body) - if (hasError) unit.warning(NoPosition, "TreeCheckers detected non-compliant trees in " + unit) } } override def newTyper(context: Context): Typer = new TreeChecker(context) class TreeChecker(context0: Context) extends Typer(context0) { - override protected def finishMethodSynthesis(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 - } + // If we don't intercept this all the synthetics get added at every phase, + // with predictably unfortunate results. + override protected def finishMethodSynthesis(templ: Template, clazz: Symbol, context: Context): Template = templ // XXX check for tree.original on TypeTrees. - private def treesDiffer(t1: Tree, t2: Tree) = - errorFn(t1.pos, "trees differ\n old: " + treestr(t1) + "\n new: " + treestr(t2)) + private def treesDiffer(t1: Tree, t2: Tree): Unit = { + def len1 = t1.toString.length + def len2 = t2.toString.length + def name = t1 match { + case t: NameTree => t.name + case _ => t1.summaryString + } + def summary = s"${t1.shortClass} $name differs, bytes $len1 -> $len2, " + errorFn(t1.pos, summary + diff(t1, t2)) + } + private def typesDiffer(tree: Tree, tp1: Type, tp2: Type) = errorFn(tree.pos, "types differ\n old: " + tp1 + "\n new: " + tp2 + "\n tree: " + tree) @@ -192,27 +253,45 @@ abstract class TreeCheckers extends Analyzer { if (t.symbol == NoSymbol) errorFn(t.pos, "no symbol: " + treestr(t)) - override def typed(tree: Tree, mode: Mode, pt: Type): Tree = returning(tree) { - case EmptyTree | TypeTree() => () - case _ if tree.tpe != null => - tpeOfTree.getOrElseUpdate(tree, try tree.tpe finally tree.clearType()) - - wrap(tree)(super.typed(tree, mode, pt) match { - case _: Literal => () - case x if x ne tree => treesDiffer(tree, x) - case _ => () - }) - case _ => () + private def passThrough(tree: Tree) = tree match { + case EmptyTree | TypeTree() => true + case _ => tree.tpe eq null + } + override def typed(tree: Tree, mode: Mode, pt: Type): Tree = ( + if (passThrough(tree)) + super.typed(tree, mode, pt) + else + checkedTyped(tree, mode, pt) + ) + private def checkedTyped(tree: Tree, mode: Mode, pt: Type): Tree = { + def tpe = try tree.tpe finally tree.clearType() + val recorded = tpeOfTree.getOrElseUpdate(tree, tpe) + val typed = wrap(tree)(super.typed(tree, mode, pt)) + + if (tree ne typed) + treesDiffer(tree, typed) + + tree } object precheck extends TreeStackTraverser { - override def traverse(tree: Tree) { - checkSymbolRefsRespectScope(tree) + private var enclosingMemberDefs: List[MemberDef] = Nil + private def pushMemberDef[T](md: MemberDef)(body: => T): T = { + enclosingMemberDefs ::= md + try body finally enclosingMemberDefs = enclosingMemberDefs.tail + } + override def traverse(tree: Tree): Unit = tree match { + case md: MemberDef => pushMemberDef(md)(traverseInternal(tree)) + case _ => traverseInternal(tree) + } + + private def traverseInternal(tree: Tree) { + checkSymbolRefsRespectScope(enclosingMemberDefs takeWhile (md => !md.symbol.hasPackageFlag), tree) checkReturnReferencesDirectlyEnclosingDef(tree) val sym = tree.symbol def accessed = sym.accessed - def fail(msg: String) = errorFn(tree.pos, msg + classstr(tree) + " / " + tree) + def fail(msg: String) = errorFn(tree.pos, msg + tree.shortClass + " / " + tree) tree match { case DefDef(_, _, _, _, _, _) => @@ -254,7 +333,7 @@ abstract class TreeCheckers extends Analyzer { case _ => } - if (tree.pos == NoPosition && tree != EmptyTree) + if (tree.canHaveAttrs && tree.pos == NoPosition) noPos(tree) else if (tree.tpe == null && phase.id > currentRun.typerPhase.id) noType(tree) @@ -281,57 +360,99 @@ abstract class TreeCheckers extends Analyzer { super.traverse(tree) } - private def checkSymbolRefsRespectScope(tree: Tree) { - def symbolOf(t: Tree): Symbol = Option(tree.symbol).getOrElse(NoSymbol) - val info = Option(symbolOf(tree).info).getOrElse(NoType) - val referencedSymbols: List[Symbol] = { - val directRef = tree match { - case _: RefTree => symbolOf(tree).toOption - case _ => None + private def checkSymbolRefsRespectScope(enclosingMemberDefs: List[MemberDef], tree: Tree) { + def symbolOf(t: Tree): Symbol = if (t.symbol eq null) NoSymbol else t.symbol + def typeOf(t: Tree): Type = if (t.tpe eq null) NoType else t.tpe + def infoOf(t: Tree): Type = symbolOf(t).info + def referencesInType(tp: Type) = tp collect { case TypeRef(_, sym, _) => sym } + def referencesInTree(t: Tree) = referencesInType(typeOf(t)) ++ referencesInType(infoOf(t)) + def symbolRefsInTree(t: Tree) = t collect { case t: RefTree => symbolOf(t) } + // Accessors are known to steal the type of the underlying field without cloning existential symbols at the new owner. + // This happens in Namer#accessorTypeCompleter. We just look the other way here. + if (symbolOf(tree).isAccessor) + return + + val treeSym = symbolOf(tree) + val treeInfo = infoOf(tree) + val treeTpe = typeOf(tree) + + def isOk(sym: Symbol) = treeSym hasTransOwner sym.safeOwner + def isEligible(sym: Symbol) = (sym ne NoSymbol) && ( + sym.isTypeParameter + || sym.isLocal + ) + val direct = tree match { + case _: RefTree => treeSym + case _ => NoSymbol + } + val referencedSymbols = (treeSym :: referencesInType(treeInfo)).distinct filter (sym => isEligible(sym) && !isOk(sym)) + def mk[T](what: String, x: T, str: T => String = (x: T) => "" + x): ((Any, String)) = + x -> s"%10s %-20s %s".format(what, classString(x), truncate(str(x), 80).trim) + + def encls = enclosingMemberDefs.filterNot(_.symbol == treeSym).zipWithIndex map { case (md, i) => mk(s"encl(${i+1})", md.symbol, signature) } + + def mkErrorMsg(outOfScope: Symbol): String = { + + def front = List( + mk[Tree]("tree", tree), + mk[Position]("position", tree.pos, posstr), + mk("with sym", treeSym, signature) + ) + def tpes = treeTpe match { + case NoType => Nil + case _ => mk[Type]("and tpe", treeTpe) :: Nil } - def referencedSyms(tp: Type) = (tp collect { - case TypeRef(_, sym, _) => sym - }).toList - val indirectRefs = referencedSyms(info) - (indirectRefs ++ directRef).distinct + def ref = mk[Symbol]("ref to", outOfScope, (s: Symbol) => s.nameString + " (" + s.debugFlagString + ")") + + val pairs = front ++ tpes ++ encls ++ (ref :: Nil) + val width = pairs.map(_._2.length).max + val fmt = "%-" + width + "s" + val lines = pairs map { + case (s: Symbol, msg) => fmt.format(msg) + " in " + ownersString(s) + case (x, msg) => fmt.format(msg) + } + lines.mkString("Out of scope symbol reference {\n", "\n", "\n}") } - for { - sym <- referencedSymbols - // Accessors are known to steal the type of the underlying field without cloning existential symbols at the new owner. - // This happens in Namer#accessorTypeCompleter. We just look the other way here. - if !tree.symbol.isAccessor - if (sym.isTypeParameter || sym.isLocal) && !(tree.symbol hasTransOwner sym.owner) - } errorFn(s"The symbol, tpe or info of tree `(${tree}) : ${info}` refers to a out-of-scope symbol, ${sym.fullLocationString}. tree.symbol.ownerChain: ${tree.symbol.ownerChain.mkString(", ")}") - } - private def checkReturnReferencesDirectlyEnclosingDef(tree: Tree) { - tree match { - case _: Return => - path.collectFirst { - case dd: DefDef => dd - } match { - case None => errorFn(s"Return node ($tree) must be enclosed in a DefDef") - case Some(dd) => - if (tree.symbol != dd.symbol) errorFn(s"Return symbol (${tree.symbol}} does not reference directly enclosing DefDef (${dd.symbol})") + referencedSymbols foreach (sym => + if (reportedAlready((tree, sym))) { + def what = tree match { + case tt: TypeTree => s"TypeTree(${tt.tpe})" + case _ => tree.shortClass + "(" + tree.symbol.nameString + ")" } - case _ => - } + } + else { + errorFn("\n" + mkErrorMsg(sym)) + reportedAlready += ((tree, sym)) + } + ) + } + + private def checkReturnReferencesDirectlyEnclosingDef(tree: Tree): Unit = tree match { + case _: Return => + path collectFirst { case dd: DefDef => dd } match { + case None => errorFn(s"Return node ($tree) must be enclosed in a DefDef") + case Some(dd) if tree.symbol != dd.symbol => errorFn(s"Return symbol (${tree.symbol}} does not reference directly enclosing DefDef (${dd.symbol})") + case _ => + } + case _ => } } object postcheck extends Traverser { - override def traverse(tree: Tree) { - tree match { - case EmptyTree | TypeTree() => () - case _ => - tpeOfTree get tree foreach { oldtpe => - if (oldtpe =:= tree.tpe) () - else typesDiffer(tree, oldtpe, tree.tpe) - - tree setType oldtpe - super.traverse(tree) - } - } + override def traverse(tree: Tree): Unit = tree match { + case EmptyTree | TypeTree() => () + case _ => + tpeOfTree get tree foreach { oldtpe => + if (tree.tpe eq null) + errorFn(s"tree.tpe=null for " + tree.shortClass + " (symbol: " + classString(tree.symbol) + " " + signature(tree.symbol) + "), last seen tpe was " + oldtpe) + else if (oldtpe =:= tree.tpe) + () + else + typesDiffer(tree, oldtpe, tree.tpe) + + super.traverse(tree setType oldtpe) + } } } } |