summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMads Hartmann Jensen <mads379@gmail.com>2013-01-26 20:01:36 +0100
committerMads Hartmann Jensen <mads379@gmail.com>2013-02-07 10:47:29 +0100
commit79e774f584d5afed72f62b0a7f93f63765d0cb99 (patch)
treed99abfc9d29b1872d8dcab325257aa61c05d889b
parent5d65772762072aa950a488c666673dc248b01d6d (diff)
downloadscala-79e774f584d5afed72f62b0a7f93f63765d0cb99.tar.gz
scala-79e774f584d5afed72f62b0a7f93f63765d0cb99.tar.bz2
scala-79e774f584d5afed72f62b0a7f93f63765d0cb99.zip
SI-7026: parseTree should never return a typed one
This commit fixes ticket SI-7026. This makes it safe to use parseTree outside of the presentation compiler thread. Solved it with an implementation that just parses the source every time without trying to memorize anything. Added tests that checks that 1. You get a new parse tree every time you ask for one. 2. You always get a parse tree, never a typed tree. 3. A parse tree should never contain any symbols or types [1]. 4. If you ask for a parse tree and then ask for a typed tree it shouldn't change the parse tree you originally asked for, i.e. property 3 still holds. Additionally the parser is now only interruptible when running on the presentation compiler thread. [1] There is an exception to this though. Some of the nodes that the compiler generates will actually contain symbols. I've chosen to just ignore these special cases for now.
-rw-r--r--src/compiler/scala/tools/nsc/interactive/CompilerControl.scala13
-rw-r--r--src/compiler/scala/tools/nsc/interactive/Global.scala7
-rw-r--r--test/files/presentation/ide-t1001326.check4
-rw-r--r--test/files/presentation/ide-t1001326/Test.scala91
-rw-r--r--test/files/presentation/ide-t1001326/src/a/A.scala5
5 files changed, 110 insertions, 10 deletions
diff --git a/src/compiler/scala/tools/nsc/interactive/CompilerControl.scala b/src/compiler/scala/tools/nsc/interactive/CompilerControl.scala
index b4af8f00d6..73414ea6ed 100644
--- a/src/compiler/scala/tools/nsc/interactive/CompilerControl.scala
+++ b/src/compiler/scala/tools/nsc/interactive/CompilerControl.scala
@@ -240,15 +240,12 @@ trait CompilerControl { self: Global =>
}
/** Returns parse tree for source `source`. No symbols are entered. Syntax errors are reported.
- * Can be called asynchronously from presentation compiler.
+ *
+ * This method is thread-safe and as such can safely run outside of the presentation
+ * compiler thread.
*/
- def parseTree(source: SourceFile): Tree = ask { () =>
- getUnit(source) match {
- case Some(unit) if unit.status >= JustParsed =>
- unit.body
- case _ =>
- new UnitParser(new CompilationUnit(source)).parse()
- }
+ def parseTree(source: SourceFile): Tree = {
+ new UnitParser(new CompilationUnit(source)).parse()
}
/** Asks for a computation to be done quickly on the presentation compiler thread */
diff --git a/src/compiler/scala/tools/nsc/interactive/Global.scala b/src/compiler/scala/tools/nsc/interactive/Global.scala
index 4ab7b98b3d..b804d45b31 100644
--- a/src/compiler/scala/tools/nsc/interactive/Global.scala
+++ b/src/compiler/scala/tools/nsc/interactive/Global.scala
@@ -225,7 +225,10 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
/** Called from parser, which signals hereby that a method definition has been parsed.
*/
override def signalParseProgress(pos: Position) {
- checkForMoreWork(pos)
+ // We only want to be interruptible when running on the PC thread.
+ if(onCompilerThread) {
+ checkForMoreWork(pos)
+ }
}
/** Called from typechecker, which signals hereby that a node has been completely typechecked.
@@ -447,7 +450,7 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
*/
@elidable(elidable.WARNING)
override def assertCorrectThread() {
- assert(initializing || (Thread.currentThread() eq compileRunner),
+ assert(initializing || onCompilerThread,
"Race condition detected: You are running a presentation compiler method outside the PC thread.[phase: %s]".format(globalPhase) +
" Please file a ticket with the current stack trace at https://www.assembla.com/spaces/scala-ide/support/tickets")
}
diff --git a/test/files/presentation/ide-t1001326.check b/test/files/presentation/ide-t1001326.check
new file mode 100644
index 0000000000..0ac15faed4
--- /dev/null
+++ b/test/files/presentation/ide-t1001326.check
@@ -0,0 +1,4 @@
+Unique OK
+Unattributed OK
+NeverModify OK
+AlwaysParseTree OK \ No newline at end of file
diff --git a/test/files/presentation/ide-t1001326/Test.scala b/test/files/presentation/ide-t1001326/Test.scala
new file mode 100644
index 0000000000..3091da4b40
--- /dev/null
+++ b/test/files/presentation/ide-t1001326/Test.scala
@@ -0,0 +1,91 @@
+import scala.tools.nsc.interactive.tests.InteractiveTest
+import scala.reflect.internal.util.SourceFile
+import scala.tools.nsc.interactive.Response
+
+object Test extends InteractiveTest {
+
+ override def execute(): Unit = {
+ val sf = sourceFiles.find(_.file.name == "A.scala").head
+ uniqueParseTree_t1001326(sf)
+ unattributedParseTree_t1001326(sf)
+ neverModifyParseTree_t1001326(sf)
+ shouldAlwaysReturnParseTree_t1001326(sf)
+ }
+
+ /**
+ * Asking twice for a parseTree on the same source should always return a new tree
+ */
+ private def uniqueParseTree_t1001326(sf: SourceFile) {
+ val parseTree1 = compiler.parseTree(sf)
+ val parseTree2 = compiler.parseTree(sf)
+ if (parseTree1 != parseTree2) {
+ reporter.println("Unique OK")
+ } else {
+ reporter.println("Unique FAILED")
+ }
+ }
+
+ /**
+ * A parseTree should never contain any symbols or types
+ */
+ private def unattributedParseTree_t1001326(sf: SourceFile) {
+ if (noSymbolsOrTypes(compiler.parseTree(sf))) {
+ reporter.println("Unattributed OK")
+ } else {
+ reporter.println("Unattributed FAILED")
+ }
+ }
+
+ /**
+ * Once you have obtained a parseTree it should never change
+ */
+ private def neverModifyParseTree_t1001326(sf: SourceFile) {
+ val parsedTree = compiler.parseTree(sf)
+ loadSourceAndWaitUntilTypechecked(sf)
+ if (noSymbolsOrTypes(parsedTree)) {
+ reporter.println("NeverModify OK")
+ } else {
+ reporter.println("NeverModify FAILED")
+ }
+ }
+
+ /**
+ * Should always return a parse tree
+ */
+ private def shouldAlwaysReturnParseTree_t1001326(sf: SourceFile) {
+ loadSourceAndWaitUntilTypechecked(sf)
+ if (noSymbolsOrTypes(compiler.parseTree(sf))) {
+ reporter.println("AlwaysParseTree OK")
+ } else {
+ reporter.println("AlwaysParseTree FAILED")
+ }
+ }
+
+ /**
+ * Load a source and block while it is type-checking.
+ */
+ private def loadSourceAndWaitUntilTypechecked(sf: SourceFile): Unit = {
+ compiler.askToDoFirst(sf)
+ val res = new Response[Unit]
+ compiler.askReload(List(sf), res)
+ res.get
+ askLoadedTyped(sf).get
+ }
+
+ /**
+ * Traverses a tree and makes sure that there are no types or symbols present in the tree with
+ * the exception of the symbol for the package 'scala'. This is because that symbol will be
+ * present in some of the nodes that the compiler generates.
+ */
+ private def noSymbolsOrTypes(tree: compiler.Tree): Boolean = {
+ tree.forAll { t =>
+ (t.symbol == null ||
+ t.symbol == compiler.NoSymbol ||
+ t.symbol == compiler.definitions.ScalaPackage // ignore the symbol for the scala package for now
+ ) && (
+ t.tpe == null ||
+ t.tpe == compiler.NoType)
+ }
+ }
+
+} \ No newline at end of file
diff --git a/test/files/presentation/ide-t1001326/src/a/A.scala b/test/files/presentation/ide-t1001326/src/a/A.scala
new file mode 100644
index 0000000000..c82ca02231
--- /dev/null
+++ b/test/files/presentation/ide-t1001326/src/a/A.scala
@@ -0,0 +1,5 @@
+package a
+
+class A {
+ def foo(s: String) = s + s
+} \ No newline at end of file