Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Harden IDE, 2nd attempt #2692

Merged
merged 6 commits into from Jun 6, 2017
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 27 additions & 1 deletion compiler/src/dotty/tools/dotc/ast/Trees.scala
Expand Up @@ -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

Expand Down Expand Up @@ -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 ||
Expand Down Expand Up @@ -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] =
Expand Down Expand Up @@ -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
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions compiler/src/dotty/tools/dotc/config/Config.scala
Expand Up @@ -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

Expand Down Expand Up @@ -163,4 +166,7 @@ object Config {
* when findMemberLimit is set.
*/
final val PendingFindMemberLimit = LogPendingFindMemberThreshold * 4

/** When in IDE, turn StaleSymbol errors into warnings instead of crashing */
final val ignoreStaleInIDE = true
}
10 changes: 6 additions & 4 deletions compiler/src/dotty/tools/dotc/core/Denotations.scala
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions compiler/src/dotty/tools/dotc/core/Mode.scala
Expand Up @@ -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")
}
8 changes: 8 additions & 0 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Such a warning will popup in the IDE (not currently because we ignore warnings without positions, but that's just because of an issue in the LSP (microsoft/language-server-protocol#249)), maybe use ctx.debug instead ?

true
}

}

object SymDenotations {
Expand Down
23 changes: 12 additions & 11 deletions compiler/src/dotty/tools/dotc/interactive/Interactive.scala
Expand Up @@ -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)
Expand Down
Expand Up @@ -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
Expand All @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also Mode.ReadPositions that we always need in the IDE, it's currently set in InteractiveDriver but could be moved here.

}
11 changes: 8 additions & 3 deletions compiler/src/dotty/tools/dotc/util/Positions.scala
Expand Up @@ -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)
Expand Down