Skip to content

Remove cruft from TyperPhase #13771

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 1 commit into from
Oct 27, 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
23 changes: 0 additions & 23 deletions compiler/src/dotty/tools/dotc/ast/Desugar.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1410,29 +1410,6 @@ object desugar {
FunctionWithMods(params, body, Modifiers(mods))
}

/** Add annotation to tree:
* tree @fullName
*
* The annotation is usually represented as a TypeTree referring to the class
* with the given name `fullName`. However, if the annotation matches a file name
* that is still to be entered, the annotation is represented as a cascade of `Selects`
* following `fullName`. This is necessary so that we avoid reading an annotation from
* the classpath that is also compiled from source.
*/
def makeAnnotated(fullName: String, tree: Tree)(using Context): Annotated = {
val parts = fullName.split('.')
val ttree = typerPhase match {
case phase: TyperPhase if phase.stillToBeEntered(parts.last) =>
val prefix =
parts.init.foldLeft(Ident(nme.ROOTPKG): Tree)((qual, name) =>
Select(qual, name.toTermName))
Select(prefix, parts.last.toTypeName)
case _ =>
TypeTree(requiredClass(fullName).typeRef)
}
Annotated(tree, New(ttree, Nil))
}

private def derivedValDef(original: Tree, named: NameTree, tpt: Tree, rhs: Tree, mods: Modifiers)(using Context) = {
val vdef = ValDef(named.name.asTermName, tpt, rhs)
.withMods(mods)
Expand Down
24 changes: 3 additions & 21 deletions compiler/src/dotty/tools/dotc/typer/TyperPhase.scala
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,6 @@ class TyperPhase(addRootImports: Boolean = true) extends Phase {

override def allowsImplicitSearch: Boolean = true

/** The contexts for compilation units that are parsed but not yet entered */
private var remaining: List[Context] = Nil

/** 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"))

// Run regardless of parsing errors
override def isRunnable(implicit ctx: Context): Boolean = true

Expand Down Expand Up @@ -68,13 +59,6 @@ class TyperPhase(addRootImports: Boolean = true) extends Phase {
JavaChecks.check(unit.tpdTree)
}


private def firstTopLevelDef(trees: List[tpd.Tree])(using Context): Symbol = trees match
case PackageDef(_, defs) :: _ => firstTopLevelDef(defs)
case Import(_, _) :: defs => firstTopLevelDef(defs)
case (tree @ TypeDef(_, _)) :: _ => tree.symbol
case _ => NoSymbol

protected def discardAfterTyper(unit: CompilationUnit)(using Context): Boolean =
unit.isJava || unit.suspended

Expand All @@ -89,11 +73,9 @@ class TyperPhase(addRootImports: Boolean = true) extends Phase {
else
newCtx

remaining = unitContexts
while remaining.nonEmpty do
Copy link
Member

@bishabosha bishabosha Oct 19, 2021

Choose a reason for hiding this comment

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

this while loop was probably added for a performance critical reason, I suggest remaining could be made a local variable (maybe premature optimisation?)

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’m pretty sure it was there to enable stillToBeEntered, which is no longer used (AFAICT).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Friendly ping @bishabosha? Is there anything I need to change here?

Copy link
Member

@bishabosha bishabosha Oct 27, 2021

Choose a reason for hiding this comment

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

Im running the benchmarks, I'm sure that it probably makes no difference if it is foreach or a loop

enterSyms(using remaining.head)
remaining = remaining.tail
val firstXmlPos = ctx.base.parserPhase match {
unitContexts.foreach(enterSyms(using _))

ctx.base.parserPhase match {
case p: ParserPhase =>
if p.firstXmlPos.exists && !defn.ScalaXmlPackageClass.exists then
report.error(
Expand Down