From 915ff94e8b0341b48d091c12ed560511b040dc62 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 5 Jun 2017 12:00:28 +0200 Subject: [PATCH 1/6] Harden withStart and friends for Positions They used to crash when applied to NoPosition. We now handle this case gracefully. --- compiler/src/dotty/tools/dotc/util/Positions.scala | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/util/Positions.scala b/compiler/src/dotty/tools/dotc/util/Positions.scala index 349933f5db7b..c98b02f06726 100644 --- a/compiler/src/dotty/tools/dotc/util/Positions.scala +++ b/compiler/src/dotty/tools/dotc/util/Positions.scala @@ -100,13 +100,18 @@ object Positions { /** A copy of this position with a different start */ def withStart(start: Int) = - fromOffsets(start, this.end, if (isSynthetic) SyntheticPointDelta else this.point - start) + if (exists) fromOffsets(start, this.end, if (isSynthetic) SyntheticPointDelta else this.point - start) + else this /** A copy of this position with a different end */ - def withEnd(end: Int) = fromOffsets(this.start, end, pointDelta) + def withEnd(end: Int) = + if (exists) fromOffsets(this.start, end, pointDelta) + else this /** A copy of this position with a different point */ - def withPoint(point: Int) = fromOffsets(this.start, this.end, point - this.start) + def withPoint(point: Int) = + if (exists) fromOffsets(this.start, this.end, point - this.start) + else this /** A synthetic copy of this position */ def toSynthetic = if (isSynthetic) this else Position(start, end) From 87f5347831abd1af9245ebb072b7bab472554c74 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 6 Jun 2017 09:25:21 +0200 Subject: [PATCH 2/6] Accept untyped children of typed trees in error cases This happens, e.g. in realApply in Applications. Change TreeAccumulator and TreeMap to survive on untyped tree nodes in error cases. Also, add a configurable check that typed nodes point to untyped ones only in error cases. --- compiler/src/dotty/tools/dotc/ast/Trees.scala | 28 ++++++++++++++++++- .../src/dotty/tools/dotc/config/Config.scala | 5 ++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Trees.scala b/compiler/src/dotty/tools/dotc/ast/Trees.scala index 12cfc5e9e0d8..f7df30b404c6 100644 --- a/compiler/src/dotty/tools/dotc/ast/Trees.scala +++ b/compiler/src/dotty/tools/dotc/ast/Trees.scala @@ -13,6 +13,7 @@ import collection.mutable.ListBuffer import parsing.Tokens.Token import printing.Printer import util.{Stats, Attachment, Property, DotClass} +import config.Config import annotation.unchecked.uncheckedVariance import language.implicitConversions @@ -113,10 +114,30 @@ object Trees { * type. (Overridden by empty trees) */ def withType(tpe: Type)(implicit ctx: Context): ThisTree[Type] = { - if (tpe.isInstanceOf[ErrorType]) assert(ctx.reporter.errorsReported) + if (tpe.isInstanceOf[ErrorType]) + assert(ctx.reporter.errorsReported) + else if (Config.checkTreesConsistent) + checkChildrenTyped(productIterator) withTypeUnchecked(tpe) } + /** Check that typed trees don't refer to untyped ones, except if + * - the parent tree is an import, or + * - the child tree is an identifier, or + * - errors were reported + */ + private def checkChildrenTyped(it: Iterator[Any])(implicit ctx: Context): Unit = + if (!this.isInstanceOf[Import[_]]) + while (it.hasNext) + it.next match { + case x: Ident[_] => // untyped idents are used in a number of places in typed trees + case x: Tree[_] => + assert(x.hasType || ctx.reporter.errorsReported, + s"$this has untyped child $x") + case xs: List[_] => checkChildrenTyped(xs.iterator) + case _ => + } + def withTypeUnchecked(tpe: Type): ThisTree[Type] = { val tree = (if (myTpe == null || @@ -1177,6 +1198,8 @@ object Trees { case Thicket(trees) => val trees1 = transform(trees) if (trees1 eq trees) tree else Thicket(trees1) + case _ if ctx.reporter.errorsReported => + tree } def transformStats(trees: List[Tree])(implicit ctx: Context): List[Tree] = @@ -1282,6 +1305,9 @@ object Trees { this(this(x, arg), annot) case Thicket(ts) => this(x, ts) + case _ if ctx.reporter.errorsReported => + // in case of errors it may be that typed trees point to untyped ones. + x } } } diff --git a/compiler/src/dotty/tools/dotc/config/Config.scala b/compiler/src/dotty/tools/dotc/config/Config.scala index 13dd45037813..94233495dbe2 100644 --- a/compiler/src/dotty/tools/dotc/config/Config.scala +++ b/compiler/src/dotty/tools/dotc/config/Config.scala @@ -72,6 +72,9 @@ object Config { /** Check positions for consistency after parsing */ final val checkPositions = true + /** Check that typed trees don't point to untyped ones */ + final val checkTreesConsistent = false + /** Show subtype traces for all deep subtype recursions */ final val traceDeepSubTypeRecursions = false @@ -163,4 +166,6 @@ object Config { * when findMemberLimit is set. */ final val PendingFindMemberLimit = LogPendingFindMemberThreshold * 4 + + final val ignoreStaleInIDE = true } From d30a95efb52966d62e653b785fd99aef73402bc4 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 6 Jun 2017 09:27:49 +0200 Subject: [PATCH 3/6] Ignore StaleSymbol errors in IDE Seems to happen in several scenarios. For now, ignore, so that we can get real work done in the IDE. Ignoring is configurable, we can turn it off if we want to hunt down problems. --- compiler/src/dotty/tools/dotc/core/Mode.scala | 3 +++ .../dotty/tools/dotc/interactive/InteractiveCompiler.scala | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/core/Mode.scala b/compiler/src/dotty/tools/dotc/core/Mode.scala index 8437dfca5436..95a4684a4b9c 100644 --- a/compiler/src/dotty/tools/dotc/core/Mode.scala +++ b/compiler/src/dotty/tools/dotc/core/Mode.scala @@ -90,4 +90,7 @@ object Mode { * In this case, identifiers should never be imported. */ val InPackageClauseName = newMode(18, "InPackageClauseName") + + /** We are in the IDE */ + val Interactive = newMode(20, "Interactive") } diff --git a/compiler/src/dotty/tools/dotc/interactive/InteractiveCompiler.scala b/compiler/src/dotty/tools/dotc/interactive/InteractiveCompiler.scala index 5f049aadb970..789940ca77e3 100644 --- a/compiler/src/dotty/tools/dotc/interactive/InteractiveCompiler.scala +++ b/compiler/src/dotty/tools/dotc/interactive/InteractiveCompiler.scala @@ -5,6 +5,7 @@ package interactive import core._ import Phases._ import typer._ +import Contexts._ class InteractiveCompiler extends Compiler { // TODO: Figure out what phases should be run in IDEs @@ -14,4 +15,7 @@ class InteractiveCompiler extends Compiler { override def phases: List[List[Phase]] = List( List(new FrontEnd) ) + + override def rootContext(implicit ctx: Context) = + super.rootContext.fresh.addMode(Mode.Interactive) } From 638a7c288badd9fd0d222ee7ecbc17b0a6c1efb8 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 6 Jun 2017 09:38:36 +0200 Subject: [PATCH 4/6] Configurable: Accept stale symbol with warning if in IDE --- compiler/src/dotty/tools/dotc/core/Denotations.scala | 10 ++++++---- .../src/dotty/tools/dotc/core/SymDenotations.scala | 8 ++++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala index 46c8c0a6a03b..f27ede8153db 100644 --- a/compiler/src/dotty/tools/dotc/core/Denotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala @@ -722,7 +722,7 @@ object Denotations { * if denotation is no longer valid. */ private def bringForward()(implicit ctx: Context): SingleDenotation = this match { - case denot: SymDenotation if ctx.stillValid(denot) => + case denot: SymDenotation if ctx.stillValid(denot) || ctx.acceptStale(denot) => assert(ctx.runId > validFor.runId || ctx.settings.YtestPickler.value, // mixing test pickler with debug printing can travel back in time s"denotation $denot invalid in run ${ctx.runId}. ValidFor: $validFor") var d: SingleDenotation = denot @@ -918,13 +918,15 @@ object Denotations { old.nextInRun = this } - def staleSymbolError(implicit ctx: Context) = { + def staleSymbolError(implicit ctx: Context) = + throw new StaleSymbol(staleSymbolMsg) + + def staleSymbolMsg(implicit ctx: Context): String = { def ownerMsg = this match { case denot: SymDenotation => s"in ${denot.owner}" case _ => "" } - def msg = s"stale symbol; $this#${symbol.id} $ownerMsg, defined in ${myValidFor}, is referred to in run ${ctx.period}" - throw new StaleSymbol(msg) + s"stale symbol; $this#${symbol.id} $ownerMsg, defined in ${myValidFor}, is referred to in run ${ctx.period}" } /** The period (interval of phases) for which there exists diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 894c8e37a987..41a788f34cdd 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -99,6 +99,14 @@ trait SymDenotations { this: Context => explain("denotation is not a SymDenotation") } } + + /** Configurable: Accept stale symbol with warning if in IDE */ + def acceptStale(denot: SingleDenotation): Boolean = + (mode.is(Mode.Interactive) && Config.ignoreStaleInIDE) && { + ctx.warning(denot.staleSymbolMsg) + true + } + } object SymDenotations { From 1eef8ddc453ffcac966f39ec9ecc6e05e21333fd Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 5 Jun 2017 11:59:47 +0200 Subject: [PATCH 5/6] Use untpd.TreeTraverser in namedTrees Avoids repeated crashes when hitting a tree that exists only in untyped form. --- .../src/dotty/tools/dotc/config/Config.scala | 1 + .../tools/dotc/interactive/Interactive.scala | 23 ++++++++++--------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/Config.scala b/compiler/src/dotty/tools/dotc/config/Config.scala index 94233495dbe2..89b9a714b268 100644 --- a/compiler/src/dotty/tools/dotc/config/Config.scala +++ b/compiler/src/dotty/tools/dotc/config/Config.scala @@ -167,5 +167,6 @@ object Config { */ final val PendingFindMemberLimit = LogPendingFindMemberThreshold * 4 + /** When in IDE, turn StaleSymbol errors into warnings instead of crashing */ final val ignoreStaleInIDE = true } diff --git a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala index a2a0047bfb2d..674e712d6499 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala @@ -131,19 +131,20 @@ object Interactive { val buf = new mutable.ListBuffer[SourceTree] trees foreach { case SourceTree(topTree, source) => - (new TreeTraverser { - override def traverse(tree: Tree)(implicit ctx: Context) = { + (new untpd.TreeTraverser { + override def traverse(tree: untpd.Tree)(implicit ctx: Context) = { tree match { - case _: Inlined => + case _: untpd.Inlined => // Skip inlined trees - case tree: NameTree - if tree.symbol.exists - && !tree.symbol.is(Synthetic) - && tree.pos.exists - && !tree.pos.isZeroExtent - && (includeReferences || isDefinition(tree)) - && treePredicate(tree) => - buf += SourceTree(tree, source) + case utree: untpd.NameTree if tree.hasType => + val tree = utree.asInstanceOf[tpd.NameTree] + if (tree.symbol.exists + && !tree.symbol.is(Synthetic) + && tree.pos.exists + && !tree.pos.isZeroExtent + && (includeReferences || isDefinition(tree)) + && treePredicate(tree)) + buf += SourceTree(tree, source) case _ => } traverseChildren(tree) From c7a5236717bb717089cbafbb803bb1792457bf65 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 6 Jun 2017 15:19:16 +0200 Subject: [PATCH 6/6] Adress reviewers comments --- compiler/src/dotty/tools/dotc/core/SymDenotations.scala | 5 ++--- .../dotty/tools/dotc/interactive/InteractiveCompiler.scala | 3 --- .../src/dotty/tools/dotc/interactive/InteractiveDriver.scala | 2 +- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 41a788f34cdd..6fa4abbcf809 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -102,11 +102,10 @@ trait SymDenotations { this: Context => /** Configurable: Accept stale symbol with warning if in IDE */ def acceptStale(denot: SingleDenotation): Boolean = - (mode.is(Mode.Interactive) && Config.ignoreStaleInIDE) && { - ctx.warning(denot.staleSymbolMsg) + mode.is(Mode.Interactive) && Config.ignoreStaleInIDE && { + ctx.echo(denot.staleSymbolMsg) true } - } object SymDenotations { diff --git a/compiler/src/dotty/tools/dotc/interactive/InteractiveCompiler.scala b/compiler/src/dotty/tools/dotc/interactive/InteractiveCompiler.scala index 789940ca77e3..6822699a4f9c 100644 --- a/compiler/src/dotty/tools/dotc/interactive/InteractiveCompiler.scala +++ b/compiler/src/dotty/tools/dotc/interactive/InteractiveCompiler.scala @@ -15,7 +15,4 @@ class InteractiveCompiler extends Compiler { override def phases: List[List[Phase]] = List( List(new FrontEnd) ) - - override def rootContext(implicit ctx: Context) = - super.rootContext.fresh.addMode(Mode.Interactive) } diff --git a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala index d47f165c93a7..c19bc0556f8a 100644 --- a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala +++ b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala @@ -33,7 +33,7 @@ class InteractiveDriver(settings: List[String]) extends Driver { override def sourcesRequired = false private val myInitCtx: Context = { - val rootCtx = initCtx.fresh.addMode(Mode.ReadPositions) + val rootCtx = initCtx.fresh.addMode(Mode.ReadPositions).addMode(Mode.Interactive) rootCtx.setSetting(rootCtx.settings.YretainTrees, true) val ctx = setup(settings.toArray, rootCtx)._2 ctx.initialize()(ctx)