Skip to content

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

Merged
merged 2 commits into from
Aug 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions compiler/src/dotty/tools/dotc/Compiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ package dotc

import core._
import Contexts._
import typer.{FrontEnd, RefChecks}
import typer.{TyperPhase, RefChecks}
import parsing.Parser
import Phases.Phase
import transform._
import dotty.tools.backend.jvm.{CollectSuperCalls, GenBCode}
Expand Down Expand Up @@ -36,7 +37,8 @@ class Compiler {

/** Phases dealing with the frontend up to trees ready for TASTY pickling */
protected def frontendPhases: List[List[Phase]] =
List(new FrontEnd) :: // Compiler frontend: scanner, parser, namer, typer
List(new Parser) :: // Compiler frontend: scanner, parser
List(new TyperPhase) :: // Compiler frontend: namer, typer
List(new YCheckPositions) :: // YCheck positions
List(new sbt.ExtractDependencies) :: // Sends information on classes' dependencies to sbt via callbacks
List(new semanticdb.ExtractSemanticDB) :: // Extract info into .semanticdb files
Expand Down
5 changes: 4 additions & 1 deletion compiler/src/dotty/tools/dotc/Run.scala
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,10 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint
val unit = ctx.compilationUnit
val prevPhase = ctx.phase.prev // can be a mini-phase
val fusedPhase = ctx.base.fusedContaining(prevPhase)
val treeString = unit.tpdTree.show(using ctx.withProperty(XprintMode, Some(())))
val tree =
if (ctx.isAfterTyper) unit.tpdTree
else unit.untpdTree
val treeString = tree.show(using ctx.withProperty(XprintMode, Some(())))

report.echo(s"result of $unit after $fusedPhase:")

Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/ast/Desugar.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import util.Spans._, Types._, Contexts._, Constants._, Names._, NameOps._, Flags
import Symbols._, StdNames._, Trees._, Phases._, ContextOps._
import Decorators._, transform.SymUtils._
import NameKinds.{UniqueName, EvidenceParamName, DefaultGetterName}
import typer.{FrontEnd, Namer, Checking}
import typer.{TyperPhase, Namer, Checking}
import util.{Property, SourceFile, SourcePosition}
import config.Feature.{sourceVersion, migrateTo3, enabled}
import config.SourceVersion._
Expand Down Expand Up @@ -1412,7 +1412,7 @@ object desugar {
def makeAnnotated(fullName: String, tree: Tree)(using Context): Annotated = {
val parts = fullName.split('.')
val ttree = typerPhase match {
case phase: FrontEnd if phase.stillToBeEntered(parts.last) =>
case phase: TyperPhase if phase.stillToBeEntered(parts.last) =>
val prefix =
parts.init.foldLeft(Ident(nme.ROOTPKG): Tree)((qual, name) =>
Select(qual, name.toTermName))
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/core/Contexts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -388,8 +388,9 @@ object Contexts {
// to be used as a default value.
compilationUnit != null && compilationUnit.isJava

/** Is current phase after FrontEnd? */
/** Is current phase after TyperPhase? */
final def isAfterTyper = base.isAfterTyper(phase)
final def isTyper = base.isTyper(phase)

/** Is this a context for the members of a class definition? */
def isClassDefContext: Boolean =
Expand Down
25 changes: 19 additions & 6 deletions compiler/src/dotty/tools/dotc/core/Phases.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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")
Copy link
Contributor Author

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"


var stop = false

Expand Down Expand Up @@ -106,7 +107,7 @@ object Phases {
phase
}
fusedPhases += phaseToAdd
val shouldAddYCheck = YCheckAfter.containsPhase(phaseToAdd) || YCheckAll
val shouldAddYCheck = filteredPhases(i).exists(_.isCheckable) && YCheckAfter.containsPhase(phaseToAdd)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Check for isCheckable.

if (shouldAddYCheck) {
val checker = new TreeChecker
fusedPhases += checker
Expand Down Expand Up @@ -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 = _
Expand All @@ -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
Expand All @@ -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])
Expand All @@ -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 {
Expand Down Expand Up @@ -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).
*/
Expand Down Expand Up @@ -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 =
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2257,7 +2257,7 @@ object Types {
if (!d.exists && !allowPrivate && ctx.mode.is(Mode.Interactive))
// In the IDE we might change a public symbol to private, and would still expect to find it.
d = memberDenot(prefix, name, true)
if (!d.exists && ctx.phaseId > FirstPhaseId && lastDenotation.isInstanceOf[SymDenotation])
if (!d.exists && ctx.isAfterTyper && lastDenotation.isInstanceOf[SymDenotation])
// name has changed; try load in earlier phase and make current
d = atPhase(ctx.phaseId - 1)(memberDenot(name, allowPrivate)).current
if (d.isOverloaded)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package interactive

import core._
import Phases._
import parsing._
import typer._

class InteractiveCompiler extends Compiler {
Expand All @@ -12,7 +13,8 @@ class InteractiveCompiler extends Compiler {
// This could be improved by reporting errors back to the IDE
// after each phase group instead of waiting for the pipeline to finish.
override def phases: List[List[Phase]] = List(
List(new FrontEnd),
List(new Parser),
List(new TyperPhase),
List(new transform.SetRootTree),
List(new transform.CookComments)
)
Expand Down
64 changes: 64 additions & 0 deletions compiler/src/dotty/tools/dotc/parsing/ParserPhase.scala
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"
}
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/Pickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I lifted that from the original PR. Without it, at least dotty.tools.dotc.CompilationTests fail.

Copy link
Member

Choose a reason for hiding this comment

The 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
Expand Down Expand Up @@ -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)
report.inform(s"typing ${unit.source}")
if (addRootImports)
newCtx.withRootImports
else
newCtx

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)
Expand All @@ -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 {
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
}
Loading