-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Separate parsing into its own Phase #13173
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,10 +13,12 @@ import scala.collection.mutable.ListBuffer | |
import dotty.tools.dotc.transform.MegaPhase._ | ||
import dotty.tools.dotc.transform._ | ||
import Periods._ | ||
import typer.{FrontEnd, RefChecks} | ||
import parsing.{ Parser} | ||
import typer.{TyperPhase, RefChecks} | ||
import typer.ImportInfo.withRootImports | ||
import ast.tpd | ||
import scala.annotation.internal.sharable | ||
import scala.util.control.NonFatal | ||
|
||
object Phases { | ||
|
||
|
@@ -64,7 +66,6 @@ object Phases { | |
YCheckAfter: List[String])(using Context): List[Phase] = { | ||
val fusedPhases = ListBuffer[Phase]() | ||
var prevPhases: Set[String] = Set.empty | ||
val YCheckAll = YCheckAfter.contains("all") | ||
|
||
var stop = false | ||
|
||
|
@@ -106,7 +107,7 @@ object Phases { | |
phase | ||
} | ||
fusedPhases += phaseToAdd | ||
val shouldAddYCheck = YCheckAfter.containsPhase(phaseToAdd) || YCheckAll | ||
val shouldAddYCheck = filteredPhases(i).exists(_.isCheckable) && YCheckAfter.containsPhase(phaseToAdd) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check for isCheckable. |
||
if (shouldAddYCheck) { | ||
val checker = new TreeChecker | ||
fusedPhases += checker | ||
|
@@ -194,6 +195,7 @@ object Phases { | |
config.println(s"nextDenotTransformerId = ${nextDenotTransformerId.toList}") | ||
} | ||
|
||
private var myParserPhase: Phase = _ | ||
private var myTyperPhase: Phase = _ | ||
private var myPostTyperPhase: Phase = _ | ||
private var mySbtExtractDependenciesPhase: Phase = _ | ||
|
@@ -215,6 +217,7 @@ object Phases { | |
private var myFlattenPhase: Phase = _ | ||
private var myGenBCodePhase: Phase = _ | ||
|
||
final def parserPhase: Phase = myParserPhase | ||
final def typerPhase: Phase = myTyperPhase | ||
final def postTyperPhase: Phase = myPostTyperPhase | ||
final def sbtExtractDependenciesPhase: Phase = mySbtExtractDependenciesPhase | ||
|
@@ -239,7 +242,8 @@ object Phases { | |
private def setSpecificPhases() = { | ||
def phaseOfClass(pclass: Class[?]) = phases.find(pclass.isInstance).getOrElse(NoPhase) | ||
|
||
myTyperPhase = phaseOfClass(classOf[FrontEnd]) | ||
myParserPhase = phaseOfClass(classOf[Parser]) | ||
myTyperPhase = phaseOfClass(classOf[TyperPhase]) | ||
myPostTyperPhase = phaseOfClass(classOf[PostTyper]) | ||
mySbtExtractDependenciesPhase = phaseOfClass(classOf[sbt.ExtractDependencies]) | ||
myPicklerPhase = phaseOfClass(classOf[Pickler]) | ||
|
@@ -262,6 +266,7 @@ object Phases { | |
} | ||
|
||
final def isAfterTyper(phase: Phase): Boolean = phase.id > typerPhase.id | ||
final def isTyper(phase: Phase): Boolean = phase.id == typerPhase.id | ||
} | ||
|
||
abstract class Phase { | ||
|
@@ -313,8 +318,8 @@ object Phases { | |
*/ | ||
def checkPostCondition(tree: tpd.Tree)(using Context): Unit = () | ||
|
||
/** Is this phase the standard typerphase? True for FrontEnd, but | ||
* not for other first phases (such as FromTasty). The predicate | ||
/** Is this phase the standard typerphase? True for TyperPhase, but | ||
* not for other first phases (such as FromTasty or Parser). The predicate | ||
* is tested in some places that perform checks and corrections. It's | ||
* different from ctx.isAfterTyper (and cheaper to test). | ||
*/ | ||
|
@@ -402,9 +407,17 @@ object Phases { | |
final def iterator: Iterator[Phase] = | ||
Iterator.iterate(this)(_.next) takeWhile (_.hasNext) | ||
|
||
final def monitor(doing: String)(body: => Unit)(using Context): Unit = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved up from FrontEnd |
||
try body | ||
catch | ||
case NonFatal(ex) => | ||
report.echo(s"exception occurred while $doing ${ctx.compilationUnit}") | ||
throw ex | ||
|
||
override def toString: String = phaseName | ||
} | ||
|
||
def parserPhase(using Context): Phase = ctx.base.parserPhase | ||
def typerPhase(using Context): Phase = ctx.base.typerPhase | ||
def postTyperPhase(using Context): Phase = ctx.base.postTyperPhase | ||
def sbtExtractDependenciesPhase(using Context): Phase = ctx.base.sbtExtractDependenciesPhase | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -573,7 +573,7 @@ object SymDenotations { | |
case _ => | ||
// Otherwise, no completion is necessary, see the preconditions of `markAbsent()`. | ||
(myInfo `eq` NoType) | ||
|| is(Invisible) && !ctx.isAfterTyper | ||
|| is(Invisible) && ctx.isTyper | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this is the intent of this line. |
||
|| is(ModuleVal, butNot = Package) && moduleClass.isAbsent(canForce) | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
package dotty.tools.dotc.parsing | ||
|
||
import dotty.tools.dotc.ast.Trees | ||
import dotty.tools.dotc.config.Config | ||
import dotty.tools.dotc.config.Printers.{ default, typr } | ||
import dotty.tools.dotc.core.Contexts.{ Context, ctx } | ||
import dotty.tools.dotc.core.Phases.Phase | ||
import dotty.tools.dotc.core.Symbols.defn | ||
import dotty.tools.dotc.typer.ImportInfo.withRootImports | ||
import dotty.tools.dotc.{ CompilationUnit, ast, report } | ||
import dotty.tools.dotc.util.{ NoSourcePosition, SourcePosition } | ||
import dotty.tools.dotc.util.Stats.record | ||
import dotty.tools.unsupported | ||
|
||
class Parser extends Phase { | ||
|
||
override def phaseName: String = Parser.name | ||
|
||
// We run TreeChecker only after type checking | ||
override def isCheckable: Boolean = false | ||
|
||
/** The position of the first XML literal encountered while parsing, | ||
* NoSourcePosition if there were no XML literals. | ||
*/ | ||
private[dotc] var firstXmlPos: SourcePosition = NoSourcePosition | ||
|
||
def parse(using Context) = monitor("parser") { | ||
val unit = ctx.compilationUnit | ||
unit.untpdTree = | ||
if (unit.isJava) new JavaParsers.JavaParser(unit.source).parse() | ||
else { | ||
val p = new Parsers.Parser(unit.source) | ||
// p.in.debugTokenStream = true | ||
val tree = p.parse() | ||
if (p.firstXmlPos.exists && !firstXmlPos.exists) | ||
firstXmlPos = p.firstXmlPos | ||
tree | ||
} | ||
|
||
val printer = if (ctx.settings.Xprint.value.contains(Parser.name)) default else typr | ||
printer.println("parsed:\n" + unit.untpdTree.show) | ||
if (Config.checkPositions) | ||
unit.untpdTree.checkPos(nonOverlapping = !unit.isJava && !ctx.reporter.hasErrors) | ||
} | ||
|
||
|
||
override def runOn(units: List[CompilationUnit])(using Context): List[CompilationUnit] = { | ||
val unitContexts = | ||
for unit <- units yield | ||
report.inform(s"parsing ${unit.source}") | ||
ctx.fresh.setCompilationUnit(unit).withRootImports | ||
|
||
unitContexts.foreach(parse(using _)) | ||
record("parsedTrees", ast.Trees.ntrees) | ||
|
||
unitContexts.map(_.compilationUnit) | ||
} | ||
|
||
def run(using Context): Unit = unsupported("run") | ||
} | ||
|
||
object Parser{ | ||
val name: String = "parser" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,7 +113,7 @@ class Pickler extends Phase { | |
val ctx2 = ctx.fresh.setSetting(ctx.settings.YreadComments, true) | ||
testUnpickler( | ||
using ctx2 | ||
.setPeriod(Period(ctx.runId + 1, FirstPhaseId)) | ||
.setPeriod(Period(ctx.runId + 1, ctx.base.typerPhase.id)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was needed to fix the pickling tests. I don't understand why. |
||
.setReporter(new ThrowingReporter(ctx.reporter)) | ||
.addMode(Mode.ReadPositions) | ||
.addMode(Mode.PrintShowExceptions)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,16 +10,22 @@ import Decorators._ | |
import ImportInfo.withRootImports | ||
import parsing.JavaParsers.JavaParser | ||
import parsing.Parsers.Parser | ||
import parsing.{Parser => ParserPhase} | ||
import config.Config | ||
import config.Printers.{typr, default} | ||
import util.Stats._ | ||
import util.{ SourcePosition, NoSourcePosition } | ||
import scala.util.control.NonFatal | ||
import ast.Trees._ | ||
|
||
class FrontEnd extends Phase { | ||
/** | ||
* | ||
* @param addRootImports Set to false in the REPL. Calling [[ImportInfo.withRootImports]] on the [[Context]] | ||
* for each [[CompilationUnit]] causes dotty.tools.repl.ScriptedTests to fail. | ||
*/ | ||
class TyperPhase(addRootImports: Boolean = true) extends Phase { | ||
|
||
override def phaseName: String = FrontEnd.name | ||
override def phaseName: String = TyperPhase.name | ||
override def isTyper: Boolean = true | ||
import ast.tpd | ||
|
||
|
@@ -28,43 +34,14 @@ class FrontEnd extends Phase { | |
/** The contexts for compilation units that are parsed but not yet entered */ | ||
private var remaining: List[Context] = Nil | ||
|
||
/** The position of the first XML literal encountered while parsing, | ||
* NoSourcePosition if there were no XML literals. | ||
*/ | ||
private var firstXmlPos: SourcePosition = NoSourcePosition | ||
|
||
/** Does a source file ending with `<name>.scala` belong to a compilation unit | ||
* that is parsed but not yet entered? | ||
*/ | ||
def stillToBeEntered(name: String): Boolean = | ||
remaining.exists(_.compilationUnit.toString.endsWith(name + ".scala")) | ||
|
||
def monitor(doing: String)(body: => Unit)(using Context): Unit = | ||
try body | ||
catch | ||
case NonFatal(ex) => | ||
report.echo(s"exception occurred while $doing ${ctx.compilationUnit}") | ||
throw ex | ||
|
||
def parse(using Context): Unit = monitor("parsing") { | ||
val unit = ctx.compilationUnit | ||
|
||
unit.untpdTree = | ||
if (unit.isJava) new JavaParser(unit.source).parse() | ||
else { | ||
val p = new Parser(unit.source) | ||
// p.in.debugTokenStream = true | ||
val tree = p.parse() | ||
if (p.firstXmlPos.exists && !firstXmlPos.exists) | ||
firstXmlPos = p.firstXmlPos | ||
tree | ||
} | ||
|
||
val printer = if (ctx.settings.Xprint.value.contains("parser")) default else typr | ||
printer.println("parsed:\n" + unit.untpdTree.show) | ||
if (Config.checkPositions) | ||
unit.untpdTree.checkPos(nonOverlapping = !unit.isJava && !ctx.reporter.hasErrors) | ||
} | ||
// Run regardless of parsing errors | ||
override def isRunnable(implicit ctx: Context): Boolean = true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems wrong, no? Then again no checkfile has failed so perhaps I'm wrong. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I lifted that from the original PR. Without it, at least There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, even in case of errors in parsing, we run the typer phase, this is particularly useful for IDEs since it means the presentation compiler is usable even when your code doesn't typecheck. |
||
|
||
def enterSyms(using Context): Unit = monitor("indexing") { | ||
val unit = ctx.compilationUnit | ||
|
@@ -103,19 +80,26 @@ class FrontEnd extends Phase { | |
override def runOn(units: List[CompilationUnit])(using Context): List[CompilationUnit] = | ||
val unitContexts = | ||
for unit <- units yield | ||
report.inform(s"compiling ${unit.source}") | ||
ctx.fresh.setCompilationUnit(unit).withRootImports | ||
unitContexts.foreach(parse(using _)) | ||
record("parsedTrees", ast.Trees.ntrees) | ||
val newCtx = ctx.fresh.setPhase(this.start).setCompilationUnit(unit) | ||
dwijnand marked this conversation as resolved.
Show resolved
Hide resolved
|
||
report.inform(s"typing ${unit.source}") | ||
if (addRootImports) | ||
newCtx.withRootImports | ||
else | ||
newCtx | ||
dwijnand marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
remaining = unitContexts | ||
while remaining.nonEmpty do | ||
enterSyms(using remaining.head) | ||
remaining = remaining.tail | ||
|
||
if firstXmlPos.exists && !defn.ScalaXmlPackageClass.exists then | ||
report.error("""To support XML literals, your project must depend on scala-xml. | ||
|See https://github.com/scala/scala-xml for more information.""".stripMargin, | ||
firstXmlPos) | ||
val firstXmlPos = ctx.base.parserPhase match { | ||
case p: ParserPhase => | ||
if p.firstXmlPos.exists && !defn.ScalaXmlPackageClass.exists then | ||
report.error( | ||
"""To support XML literals, your project must depend on scala-xml. | ||
|See https://github.com/scala/scala-xml for more information.""".stripMargin, | ||
p.firstXmlPos) | ||
case _ => | ||
} | ||
|
||
unitContexts.foreach(typeCheck(using _)) | ||
record("total trees after typer", ast.Trees.ntrees) | ||
|
@@ -128,6 +112,12 @@ class FrontEnd extends Phase { | |
def run(using Context): Unit = unsupported("run") | ||
} | ||
|
||
object FrontEnd { | ||
object TyperPhase { | ||
val name: String = "typer" | ||
} | ||
|
||
@deprecated(message = "FrontEnd has been split into TyperPhase and Parser. Refer to one or the other.") | ||
object FrontEnd { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a little sketchy, but should silence the error in the community build. |
||
// For backwards compatibility: some plugins refer to FrontEnd so that they can schedule themselves after it. | ||
val name: String = TyperPhase.name | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was redundant because containsPhase already checks for "all"