Content-Length: 1124733 | pFad | http://github.com/scala/scala/commit/335e9490254b18ceecfbad67fea2e1edcfc5ee62

DE Drop badly constrained phases with some noise · scala/scala@335e949 · GitHub
Skip to content

Commit 335e949

Browse files
committed
Drop badly constrained phases with some noise
1 parent 0975c32 commit 335e949

File tree

16 files changed

+188
-55
lines changed

16 files changed

+188
-55
lines changed

src/compiler/scala/tools/nsc/PhaseAssembly.scala

Lines changed: 98 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,10 @@ trait PhaseAssembly {
3333
phasesSet.add(terminal)
3434
reporter.warning(NoPosition, "Added default terminal phase")
3535
}
36-
val graph = DependencyGraph(phasesSet)
36+
val graph = DependencyGraph(phasesSet, warnAll = !settings.isScaladoc || settings.isDebug)
3737
for (n <- settings.genPhaseGraph.valueSetByUser; d <- settings.outputDirs.getSingleOutput if !d.isVirtual)
3838
DependencyGraph.graphToDotFile(graph, Path(d.file) / File(s"$n.dot"))
39-
graph.compilerPhaseList().tap(_ => if (!settings.isScaladoc || settings.isDebug) graph.warnings.foreach(msg => reporter.warning(NoPosition, msg)))
39+
graph.compilerPhaseList().tap(_ => graph.warnings.foreach(msg => reporter.warning(NoPosition, msg)))
4040
}
4141
}
4242

@@ -45,9 +45,11 @@ trait PhaseAssembly {
4545
* Each vertex is labeled with its phase name.
4646
*/
4747
class DependencyGraph(order: Int, start: String, val components: Map[String, SubComponent]) {
48-
import DependencyGraph.{FollowsNow, Weight}
48+
import DependencyGraph.{Follows, FollowsNow, Weight}
4949

50-
//private final val debugging = false
50+
require(components.contains(start), s"Start $start is not a component in ${components.keys.mkString(", ")}")
51+
52+
private final val debugging = false
5153

5254
private var messages: List[String] = Nil
5355
def warning(message: String): Unit = messages ::= message
@@ -80,11 +82,11 @@ class DependencyGraph(order: Int, start: String, val components: Map[String, Sub
8082
adjacency(v) ::= Edge(from = v, to = w, weight)
8183
case Some(_) if weight == FollowsNow => // retain runsRightAfter if there is a competing constraint
8284
adjacency(v) = Edge(from = v, to = w, weight) :: adjacency(v).filterNot(_.to == w)
83-
case _ =>
85+
case _ => // ignore duplicate
8486
}
8587
}
8688

87-
// input must be acyclic and only one FollowsNow edge is allowed between a pair of vertices
89+
// input must be acyclic and a vertex can have only one outgoing FollowsNow edge
8890
private def validate(): Unit = {
8991
def checkFollowsNow(v: Int): Unit =
9092
adjacency(v).foldLeft(-1) { (w, e) =>
@@ -122,6 +124,12 @@ class DependencyGraph(order: Int, start: String, val components: Map[String, Sub
122124
}
123125
}
124126
walk()
127+
// check reachability
128+
for (i <- 0.until(order) if !seen(i)) {
129+
warning(s"Phase ${names(i)} is not reachable from $start")
130+
addEdge(start, names(i), Follows)
131+
walk() // check cycles
132+
}
125133
}
126134

127135
def compilerPhaseList(): List[SubComponent] = if (order == 1) List(components(start)) else {
@@ -141,11 +149,11 @@ class DependencyGraph(order: Int, start: String, val components: Map[String, Sub
141149

142150
def dequeue(): Int = queue.dequeue().tap(v => enqueued(v) = false)
143151

144-
//def namedEdge(e: Edge): String = if (e == null) "[no edge]" else s"${names(e.from)} ${if (e.weight == FollowsNow) "=" else "-"}> ${names(e.to)}"
152+
def namedEdge(e: Edge): String = if (e == null) "[no edge]" else s"${names(e.from)}<${distance(e.from)}> ${if (e.weight == FollowsNow) "=" else "-"}> ${names(e.to)}<${distance(e.to)}>"
145153

146154
/** Remove a vertex from the queue and check outgoing edges:
147-
* if an edge improves (increases) the distance at the terminal,
148-
* record that as the new incoming edge and enqueue that vertex
155+
* if an edge improves (increases) the distance at the terminus of the edge,
156+
* record that as the new incoming edge to the terminus and enqueue that vertex
149157
* to propagate updates.
150158
*/
151159
def relax(): Unit = {
@@ -155,7 +163,7 @@ class DependencyGraph(order: Int, start: String, val components: Map[String, Sub
155163
}
156164
while (!queue.isEmpty) {
157165
val v = dequeue()
158-
//if (debugging) println(s"deq ${names(v)}")
166+
if (debugging) println(s"deq ${names(v)}")
159167
for (e <- adjacency(v)) {
160168
val w = e.to
161169
/* cannot happen as `runsRightAfter: Option[String]` is the only way to introduce a `FollowsNow`
@@ -167,12 +175,13 @@ class DependencyGraph(order: Int, start: String, val components: Map[String, Sub
167175
distance(w) = distance(v) + e.weight
168176
edgeTo(w) = e
169177
enqueue(w)
170-
//if (debugging) println(s"update ${namedEdge(e)} dist = ${distance(w)}, enq ${names(w)}")
178+
if (debugging) println(s"update ${namedEdge(e)} dist = ${distance(w)}, enq ${names(w)}")
171179
}
172180
}
173181
}
174-
//if (debugging) edgeTo.foreach(e => println(namedEdge(e)))
182+
if (debugging) edgeTo.foreach(e => println(namedEdge(e)))
175183
}
184+
176185
/** Put the vertices in a linear order.
177186
*
178187
* `Follows` edges increase the level, `FollowsNow` don't.
@@ -189,8 +198,9 @@ class DependencyGraph(order: Int, start: String, val components: Map[String, Sub
189198
/*c.internal && !d.internal ||*/ c.phaseName.compareTo(d.phaseName) < 0
190199
def sortVertex(i: Int, j: Int): Boolean = sortComponents(componentOf(i), componentOf(j))
191200

192-
distance.zipWithIndex.groupBy(_._1).toList.sortBy(_._1)
193-
.flatMap { case (_, dis) =>
201+
val di = distance.zipWithIndex // pairs (distance, node)
202+
di.groupBy(_._1).toArray.sortBy(_._1).iterator // group by distance, sorted by distance
203+
.flatMap { case (d@_, dis) =>
194204
val vs = dis.map { case (_, i) => i }
195205
val (anchors, followers) = vs.partition(v => edgeTo(v) == null || edgeTo(v).weight != FollowsNow)
196206
//if (debugging) println(s"d=$d, anchors=${anchors.toList.map(n => names(n))}, followers=${followers.toList.map(n => names(n))}")
@@ -208,7 +218,7 @@ class DependencyGraph(order: Int, start: String, val components: Map[String, Sub
208218
ends.foreach(drill(_, Nil))
209219
chains.sortWith((p, q) => sortVertex(p(0), q(0))).toList.flatten.map(componentOf)
210220
}
211-
}
221+
}.toList
212222
}
213223
validate()
214224
relax()
@@ -224,39 +234,92 @@ object DependencyGraph {
224234
final val Parser = "parser"
225235
final val Terminal = "terminal"
226236

237+
private val compilerPhases = List(
238+
"parser", "namer", "packageobjects", "typer", "superaccessors", "extmethods", "pickler",
239+
"refchecks", "patmat", "uncurry", "fields", "tailcalls", "specialize", "explicitouter",
240+
"erasure", "posterasure", "lambdalift", "constructors", "flatten", "mixin", "cleanup",
241+
"delambdafy", "jvm", "terminal",
242+
)
243+
227244
/** Create a DependencyGraph from the given phases. The graph must be acyclic.
228245
*
229246
* A component must be declared as "initial".
230247
* If no phase is "initial" but a phase is named "parser", it is taken as initial.
231248
* If no phase is "terminal" but a phase is named "terminal", it is taken as terminal.
232-
* Empty constraints are ignored.
249+
* A component phase must declare what phase it follows (runsAfter or runsRightAfter).
250+
* If there is no such phase in this run, the component is omitted with a warning.
251+
* The component may specify which phase it must precede (runsBefore), but if there is
252+
* no such phase in this run, that constraint is silently ignored.
253+
* Apparent typos of phase names incur warnings.
233254
*/
234-
def apply(phases: Iterable[SubComponent]): DependencyGraph = {
255+
def apply(components: Iterable[SubComponent], warnAll: Boolean = true): DependencyGraph = {
256+
val phases = components.toArray
257+
val phaseNames = components.iterator.map(_.phaseName).toSet
258+
def isPhaseName(s: String): Boolean = phaseNames.contains(s)
235259
val start = phases.find(_.initial)
236260
.orElse(phases.find(_.phaseName == Parser))
237261
.getOrElse(throw new AssertionError("Missing initial component"))
238262
val end = phases.find(_.terminal)
239263
.orElse(phases.find(_.phaseName == Terminal))
240264
.getOrElse(throw new AssertionError("Missing terminal component"))
241-
new DependencyGraph(phases.size, start.phaseName, phases.map(p => p.phaseName -> p).toMap).tap { graph =>
242-
for (p <- phases) {
243-
require(p.phaseName.nonEmpty, "Phase name must be non-empty.")
244-
def checkConstraint(name: String, constraint: String): Boolean =
245-
graph.components.contains(name).tap(ok => if (!ok) graph.warning(s"No phase `$name` for ${p.phaseName}.$constraint"))
246-
for (after <- p.runsRightAfter if after.nonEmpty && checkConstraint(after, "runsRightAfter"))
247-
graph.addEdge(after, p.phaseName, FollowsNow)
248-
for (after <- p.runsAfter if after.nonEmpty && !p.runsRightAfter.contains(after) && checkConstraint(after, "runsAfter"))
249-
graph.addEdge(after, p.phaseName, Follows)
250-
for (before <- p.runsBefore if before.nonEmpty && checkConstraint(before, "runsBefore"))
251-
graph.addEdge(p.phaseName, before, Follows)
252-
def noPhase(s: String): Boolean = s.isEmpty || !graph.components.contains(s)
253-
if (p != start && p != end)
254-
if (p.runsRightAfter.forall(noPhase) && p.runsAfter.forall(noPhase))
255-
graph.addEdge(start.phaseName, p.phaseName, Follows)
256-
if (p != end || p == end && p == start)
257-
if (p.runsBefore.forall(noPhase))
258-
graph.addEdge(p.phaseName, end.phaseName, Follows)
265+
val constraints = ListBuffer.empty[(String, String, Weight)]
266+
def addConstraint(from: String, to: String, weight: Weight): Unit = constraints.addOne((from, to, weight))
267+
val warnings = ListBuffer.empty[String]
268+
var i = 0
269+
while (i < phases.length) {
270+
val p = phases(i)
271+
require(p.phaseName.nonEmpty, "Phase name must be non-empty.")
272+
var constrained = false
273+
var badConstraint = false
274+
def checkConstraint(name: String, constraint: String, warn: Boolean): Boolean = isPhaseName(name).tap { ok =>
275+
if (!ok && warn) {
276+
val help = if (compilerPhases.contains(name)) "" else
277+
compilerPhases.filter(util.EditDistance.levenshtein(name, _) < 3) match {
278+
case Nil => ""
279+
case close => s" - did you mean ${util.StringUtil.oxford(close, "or")}?"
280+
}
281+
warnings.addOne(s"No phase `$name` for ${p.phaseName}.$constraint$help")
282+
badConstraint = true
283+
}
259284
}
285+
// add edges for each constraint
286+
for (after <- p.runsRightAfter if after.nonEmpty && checkConstraint(after, "runsRightAfter", warn=true)) {
287+
addConstraint(after, p.phaseName, FollowsNow)
288+
constrained = true
289+
}
290+
for (after <- p.runsAfter if after.nonEmpty && !p.runsRightAfter.contains(after) && checkConstraint(after, "runsAfter", warn=true)) {
291+
addConstraint(after, p.phaseName, Follows)
292+
constrained = true
293+
}
294+
for (before <- p.runsBefore if before.nonEmpty && checkConstraint(before, "runsBefore", warn=warnAll)) {
295+
addConstraint(p.phaseName, before, Follows)
296+
constrained = true
297+
}
298+
// if it doesn't follow a phase in the run, replace this component with a no-op that follows start
299+
if ((!constrained || badConstraint) && p != start && p != end) {
300+
phases(i) = new SubComponent {
301+
val global = p.global
302+
val phaseName = p.phaseName
303+
val runsAfter = start.phaseName :: Nil
304+
val runsRightAfter = None
305+
def newPhase(prev: Phase): Phase = new Phase(prev) {
306+
val name = phaseName
307+
def run() = ()
308+
}
309+
}
310+
addConstraint(start.phaseName, p.phaseName, Follows)
311+
if (warnAll) warnings.addOne(s"Component ${p.phaseName} dropped due to bad or missing constraint")
312+
}
313+
// terminal follows every phase; parser is not before every phase because traverse uses edgeTo to find "anchors"
314+
if (p != end || p == end && p == start)
315+
addConstraint(p.phaseName, end.phaseName, Follows)
316+
i += 1
317+
}
318+
new DependencyGraph(phases.size, start.phaseName, phases.iterator.map(p => p.phaseName -> p).toMap).tap { graph =>
319+
for ((from, to, weight) <- constraints)
320+
graph.addEdge(from, to, weight)
321+
for (warning <- warnings)
322+
graph.warning(warning)
260323
}
261324
}
262325

test/files/neg/t8755.check

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
warning: No phase `refchicks` for ploogin.runsAfter
2-
warning: No phase `jv` for ploogin.runsBefore
1+
warning: No phase `refchicks` for ploogin.runsAfter - did you mean refchecks?
2+
warning: No phase `java` for ploogin.runsBefore - did you mean jvm?
3+
warning: Component ploogin dropped due to bad or missing constraint
34
phase name id description
45
---------- -- -----------
56
parser 1 parse source into ASTs, perform simple desugaring

test/files/neg/t8755/ploogin_1.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class Ploogin(val global: Global) extends Plugin {
1616

1717
private object TestComponent extends PluginComponent {
1818
val global: Ploogin.this.global.type = Ploogin.this.global
19-
override val runsBefore = List("jv")
19+
override val runsBefore = List("java")
2020
val runsAfter = List("refchicks")
2121
val phaseName = Ploogin.this.name
2222
override def description = "A sample phase that doesn't know when to run."

test/files/neg/t8755b.check

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
warning: Phase ploogin is not reachable from parser
2+
phase name id description
3+
---------- -- -----------
4+
parser 1 parse source into ASTs, perform simple desugaring
5+
namer 2 resolve names, attach symbols to named trees
6+
packageobjects 3 load package objects
7+
typer 4 the meat and potatoes: type the trees
8+
ploogin 5 A phase that another phase must run before.
9+
superaccessors 6 add super accessors in traits and nested classes
10+
extmethods 7 add extension methods for inline classes
11+
pickler 8 serialize symbol tables
12+
refchecks 9 reference/override checking, translate nested objects
13+
patmat 10 translate match expressions
14+
uncurry 11 uncurry, translate function values to anonymous classes
15+
fields 12 synthesize accessors and fields, add bitmaps for lazy vals
16+
tailcalls 13 replace tail calls by jumps
17+
specialize 14 @specialized-driven class and method specialization
18+
explicitouter 15 this refs to outer pointers
19+
erasure 16 erase types, add interfaces for traits
20+
posterasure 17 clean up erased inline classes
21+
lambdalift 18 move nested functions to top level
22+
constructors 19 move field definitions into constructors
23+
flatten 20 eliminate inner classes
24+
mixin 21 mixin composition
25+
cleanup 22 platform-specific cleanups, generate reflective calls
26+
delambdafy 23 remove lambdas
27+
jvm 24 generate JVM bytecode
28+
terminal 25 the last phase during a compilation run

test/files/neg/t8755b/ploogin_1.scala

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
2+
package t8755
3+
4+
import scala.tools.nsc.{Global, Phase}
5+
import scala.tools.nsc.plugins.{Plugin, PluginComponent}
6+
import scala.reflect.io.Path
7+
import scala.reflect.io.File
8+
9+
/** A test plugin. */
10+
class Ploogin(val global: Global) extends Plugin {
11+
import global._
12+
13+
val name = "ploogin"
14+
val description = "A sample plugin for testing."
15+
val components = List[PluginComponent](TestComponent)
16+
17+
private object TestComponent extends PluginComponent {
18+
val global: Ploogin.this.global.type = Ploogin.this.global
19+
override val runsBefore = List("erasure")
20+
val runsAfter = Nil
21+
val phaseName = Ploogin.this.name
22+
override def description = "A phase that another phase must run before."
23+
def newPhase(prev: Phase) = new TestPhase(prev)
24+
class TestPhase(prev: Phase) extends StdPhase(prev) {
25+
override def description = TestComponent.this.description
26+
def apply(unit: CompilationUnit): Unit = ()
27+
}
28+
}
29+
}

test/files/neg/t8755b/sample_2.scala

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
//> using options -Xplugin:. -Xplugin-require:ploogin -Vphases -Werror
2+
package sample
3+
4+
// just a sample that is compiled with the sample plugin enabled
5+
object Sample extends App {
6+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<plugin>
2+
<name>ploogin</name>
3+
<classname>t8755.Ploogin</classname>
4+
</plugin>
5+

test/scaladoc/run/t8755.check

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
1-
parser,namer,packageobjects,typer,ploogin,terminal
1+
warning: No phase `refchecks` for ploogin.runsAfter
2+
parser -> namer -> packageobjects -> typer -> ploogin -> terminal
23
Done.

test/scaladoc/run/t8755/Test_2.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ object Test extends ScaladocModelTest {
1515
override def testModel(rootPackage: Package) = ()
1616

1717
override def newDocFactory =
18-
super.newDocFactory.tap(df => println(df.compiler.phaseNames.mkString(",")))
18+
super.newDocFactory.tap(df => println(df.compiler.phaseNames.mkString(" -> ")))
1919

2020
override def show() = {
2121
val xml = PluginDescription("ploogin", "t8755.Ploogin").toXML

test/scaladoc/run/t8755/ploogin_1.scala

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,13 @@ class Ploogin(val global: Global) extends Plugin {
1616

1717
private object TestComponent extends PluginComponent {
1818
val global: Ploogin.this.global.type = Ploogin.this.global
19-
//override val runsBefore = List("jvm")
2019
val runsAfter = List("refchecks")
2120
val phaseName = Ploogin.this.name
2221
override def description = "A sample phase that does so many things it's kind of hard to describe briefly."
2322
def newPhase(prev: Phase) = new TestPhase(prev)
2423
class TestPhase(prev: Phase) extends StdPhase(prev) {
2524
override def description = TestComponent.this.description
26-
def apply(unit: CompilationUnit): Unit = {
27-
// kewl kode
28-
}
25+
def apply(unit: CompilationUnit): Unit = ???
2926
}
3027
}
3128
}

test/scaladoc/run/t8755b.check

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
1-
parser,namer,packageobjects,typer,ploogin,terminal
1+
warning: No phase `refchecks` for ploogin.runsAfter
2+
parser -> namer -> packageobjects -> typer -> ploogin -> terminal
23
Done.

test/scaladoc/run/t8755b/Test_2.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ object Test extends ScaladocModelTest {
1515
override def testModel(rootPackage: Package) = ()
1616

1717
override def newDocFactory =
18-
super.newDocFactory.tap(df => println(df.compiler.phaseNames.mkString(",")))
18+
super.newDocFactory.tap(df => println(df.compiler.phaseNames.mkString(" -> ")))
1919

2020
override def show() = {
2121
val xml = PluginDescription("ploogin", "t8755.Ploogin").toXML

test/scaladoc/run/t8755b/ploogin_1.scala

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,7 @@ class Ploogin(val global: Global) extends Plugin {
2323
def newPhase(prev: Phase) = new TestPhase(prev)
2424
class TestPhase(prev: Phase) extends StdPhase(prev) {
2525
override def description = TestComponent.this.description
26-
def apply(unit: CompilationUnit): Unit = {
27-
// kewl kode
28-
}
26+
def apply(unit: CompilationUnit): Unit = ???
2927
}
3028
}
3129
}

test/scaladoc/run/t8755c.check

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,8 @@
1-
parser,namer,packageobjects,typer,ploogin,terminal
1+
warning: No phase `refchecks` for ploogin.runsBefore
2+
warning: Component ploogin dropped due to bad or missing constraint
3+
parser -> namer -> packageobjects -> typer -> ploogin -> terminal
4+
[running phase parser on newSource]
5+
[running phase namer on newSource]
6+
[running phase packageobjects on newSource]
7+
[running phase typer on newSource]
28
Done.

test/scaladoc/run/t8755c/Test_2.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,12 @@ object Test extends ScaladocModelTest {
1010
class C
1111
"""
1212

13-
override def scaladocSettings = s"-Xplugin:$testOutput -Xplugin-require:ploogin"
13+
override def scaladocSettings = s"-Xplugin:$testOutput -Xplugin-require:ploogin -Vdebug"
1414

1515
override def testModel(rootPackage: Package) = ()
1616

1717
override def newDocFactory =
18-
super.newDocFactory.tap(df => println(df.compiler.phaseNames.mkString(",")))
18+
super.newDocFactory.tap(df => println(df.compiler.phaseNames.mkString(" -> ")))
1919

2020
override def show() = {
2121
val xml = PluginDescription("ploogin", "t8755.Ploogin").toXML

0 commit comments

Comments
 (0)








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/scala/scala/commit/335e9490254b18ceecfbad67fea2e1edcfc5ee62

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy