8000 PC invalidates previous work on new run · scala/scala@27a4ae7 · GitHub
[go: up one dir, main page]
Skip to content

Commit 27a4ae7

Browse files
som-snyttsom-snytt
committed
PC invalidates previous work on new run
The "up-to-date" check is not sufficient when fetching doc comments, since a unit may have been typechecked, with doc wrapper trees removed. A test instability was due to whether the scheduler was checked by a callback or the runner thread. One sample file had no parser callbacks at all.
1 parent d835d98 commit 27a4ae7

File tree

11 files changed

+180
-87
lines changed

11 files changed

+180
-87
lines changed

‎src/compiler/scala/tools/nsc/Global.scala

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1697,10 +1697,11 @@ class Global(var currentSettings: Settings, reporter0: Reporter)
16971697
def compileLate(unit: CompilationUnit): Unit = {
16981698
addUnit(unit)
16991699

1700-
if (firstPhase ne null) { // we might get here during initialization, is a source is newer than the binary
1700+
if (firstPhase ne null) { // we might get here during initialization, if a source is newer than the binary
17011701
val maxId = math.max(globalPhase.id, typerPhase.id)
1702-
firstPhase.iterator takeWhile (_.id < maxId) foreach (ph =>
1703-
enteringPhase(ph)(ph.asInstanceOf[GlobalPhase] applyPhase unit))
1702+
firstPhase.iterator.takeWhile(_.id < maxId).foreach { ph =>
1703+
enteringPhase(ph)(ph.asInstanceOf[GlobalPhase].applyPhase(unit))
1704+
}
17041705
refreshProgress()
17051706
}
17061707
}

‎src/interactive/scala/tools/nsc/interactive/Global.scala

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
209209
/** A map that associates with each abstract file the set of responses that ware waiting
210210
* (via build) for the unit associated with the abstract file to be parsed and entered
211211
*/
212-
protected var getParsedEnteredResponses = newResponseMap
212+
protected val getParsedEnteredResponses = newResponseMap
213213

214214
private def cleanResponses(rmap: ResponseMap): Unit = {
215215
for ((source, rs) <- rmap.toList) {
@@ -301,6 +301,7 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
301301
def isOutOfDate: Boolean = outOfDate
302302

303303
def demandNewCompilerRun() = {
304+
if (!lastWasReload) allSources.foreach(getUnit(_).foreach(reset(_)))
304305
if (outOfDate) throw new FreshRunReq // cancel background compile
305306
else outOfDate = true // proceed normally and enable new background compile
306307
}
@@ -429,13 +430,14 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
429430
* @param pos The position of the tree if polling while typechecking, NoPosition otherwise
430431
*
431432
*/
433+
@nowarn("cat=lint-nonlocal-return")
432434
private[interactive] def pollForWork(pos: Position): Unit = {
433435
var loop: Boolean = true
434436
while (loop) {
435-
breakable{
437+
breakable {
436438
loop = false
437439
// TODO refactor to eliminate breakable/break/return?
438-
(if (!interruptsEnabled) return): @nowarn("cat=lint-nonlocal-return")
440+
if (!interruptsEnabled) return
439441
if (pos == NoPosition || nodesSeen % yieldPeriod == 0)
440442
Thread.`yield`()
441443

@@ -448,7 +450,7 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
448450
case Some(WorkEvent(id, _)) =>
449451
debugLog("some work at node "+id+" current = "+nodesSeen)
450452
// assert(id >= nodesSeen)
451-
moreWorkAtNode = id
453+
moreWorkAtNode = id
452454
case None =>
453455
}
454456

@@ -464,7 +466,8 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
464466
debugLog("ask finished"+timeStep)
465467
interruptsEnabled = true
466468
}
467-
loop = true; break()
469+
loop = true
470+
break()
468471
case _ =>
469472
}
470473

@@ -475,8 +478,8 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
475478
logreplay("exception thrown", scheduler.pollThrowable()) match {
476479
case Some(ex: FreshRunReq) =>
477480
newTyperRun()
478-
minRunId = currentRunId
479-
demandNewCompilerRun()
481+
minRunId = currentRunId
482+
demandNewCompilerRun()
480483

481484
case Some(ShutdownReq) =>
482485
scheduler.synchronized { // lock the work queue so no more items are posted while we clean it up
@@ -498,7 +501,9 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
498501
throw ShutdownReq
499502
}
500503

501-
case Some(ex: Throwable) => log.flush(); throw ex
504+
case Some(ex: Throwable) =>
505+
log.flush()
506+
throw ex
502507
case _ =>
503508
}
504509

@@ -563,7 +568,7 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
563568
reporter.reset()
564569

565570
// remove any files in first that are no longer maintained by presentation compiler (i.e. closed)
566-
allSources = allSources filter (s => unitOfFile contains (s.file))
571+
allSources = allSources.filter(s => unitOfFile.contains(s.file))
567572

568573
// ensure all loaded units are parsed
569574
for (s <- allSources; unit <- getUnit(s)) {
@@ -583,7 +588,7 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
583588
}
584589

585590
// ensure all loaded units are typechecked
586-
for (s <- allSources; if !ignoredFiles(s.file); unit <- getUnit(s)) {
591+
for (s <- allSources if !ignoredFiles(s.file); unit <- getUnit(s))
587592
try {
588593
if (!unit.isUpToDate)
589594
if (unit.problems.isEmpty || !settings.YpresentationStrict.value)
@@ -611,7 +616,6 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
611616

612617
reporter.error(unit.body.pos, "Presentation compiler crashed while type checking this file: %s".format(ex.toString()))
613618
}
614-
}
615619

616620
// move units removable after this run to the "to-be-removed" buffer
617621
toBeRemoved.synchronized {
@@ -720,9 +724,12 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
720724
val result = results.head
721725
results = results.tail
722726
if (results.isEmpty) {
723-
response set result
727+
response.set(result)
724728
debugLog("responded"+timeStep)
725-
} else response setProvisionally result
729+
}
730+
else {
731+
response.setProvisionally(result)
732+
}
726733
}
727734
}
728735
} catch {
@@ -860,15 +867,13 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
860867

861868
private def withTempUnits[T](sources: List[SourceFile])(f: (SourceFile => RichCompilationUnit) => T): T = {
862869
val unitOfSrc: SourceFile => RichCompilationUnit = src => unitOfFile(src.file)
863-
sources filterNot (getUnit(_).isDefined) match {
870+
sources.filterNot(getUnit(_).isDefined) match {
864871
case Nil =>
865872
f(unitOfSrc)
866873
case unknown =>
867874
reloadSources(unknown)
868-
try {
869-
f(unitOfSrc)
870-
} finally
871-
afterRunRemoveUnitsOf(unknown)
875+
try f(unitOfSrc)
876+
finally afterRunRemoveUnitsOf(unknown)
872877
}
873878
}
874879

@@ -949,21 +954,21 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
949954
}
950955

951956
/** Implements CompilerControl.askDocComment */
952-
private[interactive] def getDocComment(sym: Symbol, source: SourceFile, site: Symbol, fragments: List[(Symbol,SourceFile)],
957+
private[interactive] def getDocComment(sym: Symbol, source: SourceFile, site: Symbol,
958+
fragments: List[(Symbol, SourceFile)],
953959
response: Response[(String, String, Position)]): Unit = {
954960
informIDE(s"getDocComment $sym at $source, site $site")
955961
respond(response) {
956-
withTempUnits(fragments.unzip._2){ units =>
957-
for((sym, src) <- fragments) {
958-
val mirror = findMirrorSymbol(sym, units(src))
959-
if (mirror ne NoSymbol) forceDocComment(mirror, units(src))
962+
withTempUnits(fragments.unzip._2) { unitForSrc =>
963+
for ((sym, src) <- fragments) {
964+
val mirror = findMirrorSymbol(sym, unitForSrc(src))
965+
if (mirror ne NoSymbol) forceDocComment(mirror, unitForSrc(src))
960966
}
961-
val mirror = findMirrorSymbol(sym, units(source))
967+
val mirror = findMirrorSymbol(sym, unitForSrc(source))
962968
if (mirror eq NoSymbol)
963969
("", "", NoPosition)
964-
else {
970+
else
965971
(expandedDocComment(mirror, site), rawDocComment(mirror), docCommentPos(mirror))
966-
}
967972
}
968973
}
969974
// New typer run to remove temp units and drop per-run caches that might refer to symbols entered from temp units.

‎src/interactive/scala/tools/nsc/interactive/Response.scala

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,4 +112,13 @@ class Response[T] {
112112
complete = false
113113
cancelled = false
114114
}
115+
116+
override def toString =
117+
if (!isComplete) "Response incomplete"
118+
else if (isCancelled) "Response cancelled"
119+
else get(0L) match {
120+
case None => "Response has no provisional result"
121+
case Some(Right(t)) => s"Response failed: ${t.getMessage}"
122+
case Some(Left(data)) => s"Response data: ${data}"
123+
}
115124
}

‎src/interactive/scala/tools/nsc/interactive/tests/InteractiveTest.scala

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,19 +75,15 @@ abstract class InteractiveTest
7575
protected def ++(tests: PresentationCompilerTestDef*): Unit = testActions ++= tests
7676

7777
/** Test's entry point */
78-
def main(args: Array[String]): Unit = {
79-
try execute()
80-
finally askShutdown()
81-
}
78+
def main(args: Array[String]): Unit = try execute() finally askShutdown()
8279

83-
protected def execute(): Unit = {
80+
protected def execute(): Unit =
8481
util.stringFromStream { ostream =>
8582
Console.withOut(ostream) {
8683
loadSources()
8784
runDefaultTests()
8885
}
8986
}.linesIterator.map(normalize).foreach(println)
90-
}
9187

9288
protected def normalize(s: String) = s
9389

‎src/interactive/scala/tools/nsc/interactive/tests/core/AskCommand.scala

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,14 @@ trait AskParse extends AskCommand {
5353
/** `sources` need to be entirely parsed before running the test
5454
* (else commands such as `AskTypeCompletionAt` may fail simply because
5555
* the source's AST is not yet loaded).
56+
*
57+
* Submit each parse job and get the response sequentially;
58+
* otherwise, parser progress update will run the next job.
5659
*/
57-
def askParse(sources: Seq[SourceFile]): Unit = {
58-
val responses = sources map (askParse(_))
59-
responses.foreach(_.get) // force source files parsing
60-
}
60+
def askParse(sources: Seq[SourceFile]): Unit =
61+
for (source <- sources) {
62+
val _ = askParse(source).get
63+
}
6164

6265
private def askParse(src: SourceFile, keepLoaded: Boolean = true): Response[Tree] = {
6366
ask {

‎test/files/presentation/doc/doc.scala

Lines changed: 15 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
//> using options -Xlint -Werror
2-
import scala.reflect.internal.util.{ BatchSourceFile, SourceFile }
3-
import scala.tools.nsc.doc
4-
import scala.tools.nsc.doc.base._
5-
import scala.tools.nsc.doc.base.comment._
2+
import scala.reflect.internal.util.{BatchSourceFile, SourceFile}
3+
import scala.tools.nsc.doc.{ScaladocAnalyzer, ScaladocGlobalTrait}
4+
import scala.tools.nsc.doc.base.{CommentFactoryBase, LinkTo, MemberLookupBase}
5+
import scala.tools.nsc.doc.base.comment.{Body, Comment}
66
import scala.tools.nsc.interactive._
77
import scala.tools.nsc.interactive.tests._
88

@@ -38,17 +38,17 @@ object Test extends InteractiveTest {
3838
prepre + docComment(nTags) + prepost + post
3939
}
4040

41-
override lazy val compiler: Global { def getComment(sym: Symbol, source: SourceFile, fragments: List[(Symbol,SourceFile)]): Option[Comment] } = {
41+
override lazy val compiler: Global { def getComment(sym: Symbol, source: SourceFile, fragments: List[(Symbol, SourceFile)]): Option[Comment] } = {
4242
prepareSettings(settings)
43-
new Global(settings, compilerReporter) with MemberLookupBase with CommentFactoryBase with doc.ScaladocGlobalTrait {
43+
new Global(settings, compilerReporter) with MemberLookupBase with CommentFactoryBase with ScaladocGlobalTrait {
4444
outer =>
4545

4646
val global: this.type = this
4747

4848
@annotation.nowarn
4949
override lazy val analyzer = new {
5050
val global: outer.type = outer
51-
} with doc.ScaladocAnalyzer with InteractiveAnalyzer {
51+
} with ScaladocAnalyzer with InteractiveAnalyzer {
5252
override def newTyper(context: Context): InteractiveTyper with ScaladocTyper =
5353
new Typer(context) with InteractiveTyper with ScaladocTyper
5454
}
@@ -59,7 +59,7 @@ object Test extends InteractiveTest {
5959
def warnNoLink = false
6060
def findExternalLink(sym: Symbol, name: String) = None
6161

62-
def getComment(sym: Symbol, source: SourceFile, fragments: List[(Symbol,SourceFile)]): Option[Comment] = {
62+
def getComment(sym: Symbol, source: SourceFile, fragments: List[(Symbol, SourceFile)]): Option[Comment] = {
6363
val docResponse = new Response[(String, String, Position)]
6464
askDocComment(sym, source, sym.owner, fragments, docResponse)
6565
docResponse.get.swap.toOption flatMap {
@@ -74,12 +74,11 @@ object Test extends InteractiveTest {
7474
}
7575

7676
override def runDefaultTests(): Unit = {
77-
import compiler._
77+
import compiler.{NoSymbol, TermName, Tree, TypeName, askParsedEntered, getComment}
7878
def findSource(name: String) = sourceFiles.find(_.file.name == name).get
7979

8080
val className = names.head
81-
for (name <- names;
82-
i <- 1 to tags.length) {
81+
for (name <- names; i <- 1 to tags.length) {
8382
val newText = text(name, i)
8483
val source = findSource("Class.scala")
8584
val batch = new BatchSourceFile(source.file, newText.toCharArray)
@@ -100,12 +99,13 @@ object Test extends InteractiveTest {
10099
if (toplevel eq NoSymbol) {
101100
val clazz = compiler.rootMirror.EmptyPackage.info.decl(TypeName(className))
102101
val term = clazz.info.decl(TermName(name))
103-
if (term eq NoSymbol) clazz.info.decl(TypeName(name)) else
104-
if (term.isAccessor) term.accessed else term
105-
} else toplevel
102+
if (term eq NoSymbol) clazz.info.decl(TypeName(name))
103+
else if (term.isAccessor) term.accessed else term
104+
}
105+
else toplevel
106106
}
107107

108-
getComment(sym, batch, (sym,batch)::Nil) match {
108+
getComment(sym, batch, sym -> batch :: Nil) match {
109109
case None => println(s"Got no doc comment for $name")
110110
case Some(comment) =>
111111
import comment._
@@ -117,38 +117,5 @@ object Test extends InteractiveTest {
117117
}
118118
}
119119
}
120-
121-
// The remainder of this test has been found to fail intermittently on Windows
122-
// only. The problem is difficult to isolate and reproduce; see
123-
// https://github.com/scala/scala-dev/issues/72 for details.
124-
// So if we're on Windows, let's just bail out here.
125-
if (scala.util.Properties.isWin) return
126-
127-
// Check inter-classes documentation one-time retrieved ok.
128-
val baseSource = findSource("Base.scala")
129-
val derivedSource = findSource("Derived.scala")
130-
def existsText(where: Any, text: String): Boolean = where match {
131-
case s: String => s contains text
132-
case s: Seq[_] => s exists (existsText(_, text))
133-
case p: Product => p.productIterator exists (existsText(_, text))
134-
case c: Comment => existsText(c.body, text)
135-
case x => throw new MatchError(x)
136-
}
137-
val (derived, base) = compiler.ask { () =>
138-
val derived = compiler.rootMirror.RootPackage.info.decl(newTermName("p")).info.decl(newTypeName("Derived"))
139-
(derived, derived.ancestors(0))
140-
}
141-
val cmt1 = getComment(derived, derivedSource, (base, baseSource)::(derived, derivedSource)::Nil)
142-
if (!existsText(cmt1, "This is Derived comment"))
143-
println("Unexpected Derived class comment:"+cmt1)
144-
145-
val (fooDerived, fooBase) = compiler.ask { () =>
146-
val decl = derived.tpe.decl(newTermName("foo"))
147-
(decl, decl.allOverriddenSymbols(0))
148-
}
149-
150-
val cmt2 = getComment(fooDerived, derivedSource, (fooBase, baseSource)::(fooDerived, derivedSource)::Nil)
151-
if (!existsText(cmt2, "Base method has documentation"))
152-
println("Unexpected foo method comment:"+cmt2)
153120
}
154121
}

‎test/files/presentation/dock.check

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
reload: Base.scala, Class.scala, Derived.scala

0 commit comments

Comments
 (0)
0