From b1aee825b89af9921327eea19d96e75ed11e8f95 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 26 Jan 2022 22:02:55 +0100 Subject: [PATCH 1/4] Keep language imports around longer Some phases might need to know what language imports are active. Concretely, uner explicit nulls, the exhaustivity checks for patterns need to know that. We therefore keep language imports until phase PruneErasedDefs. Other imports are dropped in FirstTransform, as before. The handling of statements in MegaPhase was changed so that imports now are visible in the context for transforming the following statements. Fixes #14346 --- .../tools/dotc/transform/FirstTransform.scala | 8 +++-- .../tools/dotc/transform/MegaPhase.scala | 34 +++++++++++++++++-- .../dotc/transform/PruneErasedDefs.scala | 10 ++++++ .../src/dotty/tools/dotc/typer/Typer.scala | 2 +- 4 files changed, 47 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/FirstTransform.scala b/compiler/src/dotty/tools/dotc/transform/FirstTransform.scala index 1ff9edda16d8..03ab3860a127 100644 --- a/compiler/src/dotty/tools/dotc/transform/FirstTransform.scala +++ b/compiler/src/dotty/tools/dotc/transform/FirstTransform.scala @@ -25,7 +25,8 @@ object FirstTransform { } /** The first tree transform - * - eliminates some kinds of trees: Imports, NamedArgs + * - eliminates some kinds of trees: Imports other than language imports, + * Exports, NamedArgs, type trees other than TypeTree * - stubs out native methods * - eliminates self tree in Template and self symbol in ClassInfo * - collapses all type trees to trees of class TypeTree @@ -58,7 +59,7 @@ class FirstTransform extends MiniPhase with InfoTransformer { thisPhase => tree.symbol.is(JavaStatic) && qualTpe.derivesFrom(tree.symbol.enclosingClass), i"non member selection of ${tree.symbol.showLocated} from ${qualTpe} in $tree") case _: TypeTree => - case _: Import | _: NamedArg | _: TypTree => + case _: Export | _: NamedArg | _: TypTree => assert(false, i"illegal tree: $tree") case _ => } @@ -136,7 +137,8 @@ class FirstTransform extends MiniPhase with InfoTransformer { thisPhase => } override def transformOther(tree: Tree)(using Context): Tree = tree match { - case tree: ImportOrExport => EmptyTree + case tree: Import if untpd.languageImport(tree.expr).isEmpty => EmptyTree + case tree: Export => EmptyTree case tree: NamedArg => transformAllDeep(tree.arg) case tree => if (tree.isType) toTypeTree(tree) else tree } diff --git a/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala b/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala index 77671fc6498c..a7a19d03e9ff 100644 --- a/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala +++ b/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala @@ -432,16 +432,44 @@ class MegaPhase(val miniPhases: Array[MiniPhase]) extends Phase { def transformSpecificTree[T <: Tree](tree: T, start: Int)(using Context): T = transformTree(tree, start).asInstanceOf[T] - def transformStats(trees: List[Tree], exprOwner: Symbol, start: Int)(using Context): List[Tree] = { + def transformStats(trees: List[Tree], exprOwner: Symbol, start: Int)(using Context): List[Tree] = + def transformStat(stat: Tree)(using Context): Tree = stat match { case _: Import | _: DefTree => transformTree(stat, start) case Thicket(stats) => cpy.Thicket(stat)(stats.mapConserve(transformStat)) case _ => transformTree(stat, start)(using ctx.exprContext(stat, exprOwner)) } + + def restContext(tree: Tree)(using Context): Context = tree match + case imp: Import => ctx.importContext(imp, imp.symbol) + case _ => ctx + + // A slower implementation that avoids deep recursions and stack overflows + def transformSlow(trees: List[Tree])(using Context): List[Tree] = + var curCtx = ctx + flatten(trees.mapConserve { tree => + val tree1 = transformStat(tree)(using curCtx) + curCtx = restContext(tree)(using curCtx) + tree1 + }) + + def recur(trees: List[Tree], count: Int)(using Context): List[Tree] = + if count > MapRecursionLimit then + transformSlow(trees) + else trees match + case tree :: rest => + val tree1 = transformStat(tree) + val rest1 = recur(rest, count + 1)(using restContext(tree)) + if (tree1 eq tree) && (rest1 eq rest) then trees + else tree1 match + case Thicket(elems1) => elems1 ::: rest1 + case _ => tree1 :: rest1 + case nil => nil + val nestedCtx = prepStats(trees, start) - val trees1 = trees.mapInline(transformStat(_)(using nestedCtx)) + val trees1 = recur(trees, 0)(using nestedCtx) goStats(trees1, start)(using nestedCtx) - } + end transformStats def transformUnit(tree: Tree)(using Context): Tree = { val nestedCtx = prepUnit(tree, 0) diff --git a/compiler/src/dotty/tools/dotc/transform/PruneErasedDefs.scala b/compiler/src/dotty/tools/dotc/transform/PruneErasedDefs.scala index 7d8f0025eb15..e52b77379c27 100644 --- a/compiler/src/dotty/tools/dotc/transform/PruneErasedDefs.scala +++ b/compiler/src/dotty/tools/dotc/transform/PruneErasedDefs.scala @@ -14,6 +14,7 @@ import StdNames.nme import ast.tpd import SymUtils._ import config.Feature +import Decorators.* /** This phase makes all erased term members of classes private so that they cannot * conflict with non-erased members. This is needed so that subsequent phases like @@ -21,6 +22,7 @@ import config.Feature * The phase also replaces all expressions that appear in an erased context by * default values. This is necessary so that subsequent checking phases such * as IsInstanceOfChecker don't give false negatives. + * Finally, the phase drops (language-) imports. */ class PruneErasedDefs extends MiniPhase with SymTransformer { thisTransform => import tpd._ @@ -54,10 +56,18 @@ class PruneErasedDefs extends MiniPhase with SymTransformer { thisTransform => checkErasedInExperimental(tree.symbol) tree + override def transformOther(tree: Tree)(using Context): Tree = tree match + case tree: Import => EmptyTree + case _ => tree + def checkErasedInExperimental(sym: Symbol)(using Context): Unit = // Make an exception for Scala 2 experimental macros to allow dual Scala 2/3 macros under non experimental mode if sym.is(Erased, butNot = Macro) && sym != defn.Compiletime_erasedValue && !sym.isInExperimentalScope then Feature.checkExperimentalFeature("erased", sym.sourcePos) + + override def checkPostCondition(tree: Tree)(using Context): Unit = tree match + case _: tpd.Import => assert(false, i"illegal tree: $tree") + case _ => } object PruneErasedDefs { diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 4c0c10f7c026..02f3f6b4f164 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2556,7 +2556,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer |The selector is not a member of an object or package.""") else typd(imp.expr, AnySelectionProto) - def typedImport(imp: untpd.Import, sym: Symbol)(using Context): Import = + def typedImport(imp: untpd.Import, sym: Symbol)(using Context): Tree = val expr1 = typedImportQualifier(imp, typedExpr(_, _)(using ctx.withOwner(sym))) checkLegalImportPath(expr1) val selectors1 = typedSelectors(imp.selectors) From 1bc6cb943896a916ee99337d3d1dac336d98bdc0 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 28 Jan 2022 10:17:24 +0100 Subject: [PATCH 2/4] Make import contexts visible in macro transforms Todo: We have very similar and involved operations now sor handling statements in tpd.TreeMapWithPreciseStatContexts and MegaPhase. Can we factor out the common logic? But we should not create any closures doing so, to keep things fast. --- .../tools/dotc/ast/TreeMapWithImplicits.scala | 41 +---------------- compiler/src/dotty/tools/dotc/ast/tpd.scala | 46 ++++++++++++++++++- .../tools/dotc/transform/MacroTransform.scala | 14 ++---- 3 files changed, 49 insertions(+), 52 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/TreeMapWithImplicits.scala b/compiler/src/dotty/tools/dotc/ast/TreeMapWithImplicits.scala index 359f5d5c2580..3f4ff4687787 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeMapWithImplicits.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeMapWithImplicits.scala @@ -15,51 +15,12 @@ import scala.annotation.tailrec * * This incudes implicits defined in scope as well as imported implicits. */ -class TreeMapWithImplicits extends tpd.TreeMap { +class TreeMapWithImplicits extends tpd.TreeMapWithPreciseStatContexts { import tpd._ def transformSelf(vd: ValDef)(using Context): ValDef = cpy.ValDef(vd)(tpt = transform(vd.tpt)) - /** Transform statements, while maintaining import contexts and expression contexts - * in the same way as Typer does. The code addresses additional concerns: - * - be tail-recursive where possible - * - don't re-allocate trees where nothing has changed - */ - override def transformStats(stats: List[Tree], exprOwner: Symbol)(using Context): List[Tree] = { - - @tailrec def traverse(curStats: List[Tree])(using Context): List[Tree] = { - - def recur(stats: List[Tree], changed: Tree, rest: List[Tree])(using Context): List[Tree] = - if (stats eq curStats) { - val rest1 = transformStats(rest, exprOwner) - changed match { - case Thicket(trees) => trees ::: rest1 - case tree => tree :: rest1 - } - } - else stats.head :: recur(stats.tail, changed, rest) - - curStats match { - case stat :: rest => - val statCtx = stat match { - case stat: DefTree => ctx - case _ => ctx.exprContext(stat, exprOwner) - } - val restCtx = stat match { - case stat: Import => ctx.importContext(stat, stat.symbol) - case _ => ctx - } - val stat1 = transform(stat)(using statCtx) - if (stat1 ne stat) recur(stats, stat1, rest)(using restCtx) - else traverse(rest)(using restCtx) - case nil => - stats - } - } - traverse(stats) - } - private def nestedScopeCtx(defs: List[Tree])(using Context): Context = { val nestedCtx = ctx.fresh.setNewScope defs foreach { diff --git a/compiler/src/dotty/tools/dotc/ast/tpd.scala b/compiler/src/dotty/tools/dotc/ast/tpd.scala index 1334a4f73016..f0db2533bfdb 100644 --- a/compiler/src/dotty/tools/dotc/ast/tpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/tpd.scala @@ -1153,10 +1153,54 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { recur(trees, 0) end extension + /** A treemap that generates the same contexts as the original typer for statements. + * This means: + * - statements that are not definitions get the exprOwner as owner + * - imports are reflected in the contexts of subsequent statements + */ + class TreeMapWithPreciseStatContexts(cpy: TreeCopier = tpd.cpy) extends TreeMap(cpy): + + /** Transform statements, while maintaining import contexts and expression contexts + * in the same way as Typer does. The code addresses additional concerns: + * - be tail-recursive where possible + * - don't re-allocate trees where nothing has changed + */ + override def transformStats(stats: List[Tree], exprOwner: Symbol)(using Context): List[Tree] = + + @tailrec def traverse(curStats: List[Tree])(using Context): List[Tree] = + + def recur(stats: List[Tree], changed: Tree, rest: List[Tree])(using Context): List[Tree] = + if stats eq curStats then + val rest1 = transformStats(rest, exprOwner) + changed match + case Thicket(trees) => trees ::: rest1 + case tree => tree :: rest1 + else + stats.head :: recur(stats.tail, changed, rest) + + curStats match + case stat :: rest => + val statCtx = stat match + case _: DefTree | _: ImportOrExport => ctx + case _ => ctx.exprContext(stat, exprOwner) + val restCtx = stat match + case stat: Import => ctx.importContext(stat, stat.symbol) + case _ => ctx + val stat1 = transform(stat)(using statCtx) + if stat1 ne stat then recur(stats, stat1, rest)(using restCtx) + else traverse(rest)(using restCtx) + case nil => + stats + + traverse(stats) + end transformStats + + end TreeMapWithPreciseStatContexts + /** Map Inlined nodes, NamedArgs, Blocks with no statements and local references to underlying arguments. * Also drops Inline and Block with no statements. */ - class MapToUnderlying extends TreeMap { + private class MapToUnderlying extends TreeMap { override def transform(tree: Tree)(using Context): Tree = tree match { case tree: Ident if isBinding(tree.symbol) && skipLocal(tree.symbol) => tree.symbol.defTree match { diff --git a/compiler/src/dotty/tools/dotc/transform/MacroTransform.scala b/compiler/src/dotty/tools/dotc/transform/MacroTransform.scala index 6a246e509e61..0baf85c4e21d 100644 --- a/compiler/src/dotty/tools/dotc/transform/MacroTransform.scala +++ b/compiler/src/dotty/tools/dotc/transform/MacroTransform.scala @@ -29,19 +29,11 @@ abstract class MacroTransform extends Phase { */ protected def transformPhase(using Context): Phase = this - class Transformer extends TreeMap(cpy = cpyBetweenPhases) { + class Transformer extends TreeMapWithPreciseStatContexts(cpy = cpyBetweenPhases): - protected def localCtx(tree: Tree)(using Context): FreshContext = + protected def localCtx(tree: Tree)(using Context): FreshContext = ctx.fresh.setTree(tree).setOwner(localOwner(tree)) - override def transformStats(trees: List[Tree], exprOwner: Symbol)(using Context): List[Tree] = { - def transformStat(stat: Tree): Tree = stat match { - case _: Import | _: DefTree => transform(stat) - case _ => transform(stat)(using ctx.exprContext(stat, exprOwner)) - } - flatten(trees.mapconserve(transformStat(_))) - } - override def transform(tree: Tree)(using Context): Tree = try tree match { @@ -67,5 +59,5 @@ abstract class MacroTransform extends Phase { def transformSelf(vd: ValDef)(using Context): ValDef = cpy.ValDef(vd)(tpt = transform(vd.tpt)) - } + end Transformer } From 9c668f330857eecf00da603c21d34d0df66cd527 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 28 Jan 2022 13:00:07 +0100 Subject: [PATCH 3/4] A version of transformStats that avoids stack overflows --- compiler/src/dotty/tools/dotc/ast/tpd.scala | 41 +++++++++++---------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/tpd.scala b/compiler/src/dotty/tools/dotc/ast/tpd.scala index f0db2533bfdb..3e63f14e2634 100644 --- a/compiler/src/dotty/tools/dotc/ast/tpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/tpd.scala @@ -1164,37 +1164,38 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { * in the same way as Typer does. The code addresses additional concerns: * - be tail-recursive where possible * - don't re-allocate trees where nothing has changed + * - avoid stack overflows for long statement lists */ override def transformStats(stats: List[Tree], exprOwner: Symbol)(using Context): List[Tree] = - - @tailrec def traverse(curStats: List[Tree])(using Context): List[Tree] = - - def recur(stats: List[Tree], changed: Tree, rest: List[Tree])(using Context): List[Tree] = - if stats eq curStats then - val rest1 = transformStats(rest, exprOwner) - changed match - case Thicket(trees) => trees ::: rest1 - case tree => tree :: rest1 - else - stats.head :: recur(stats.tail, changed, rest) - - curStats match + @tailrec + def loop(mapped: mutable.ListBuffer[Tree] | Null, unchanged: List[Tree], pending: List[Tree])(using Context): List[Tree] = + pending match case stat :: rest => - val statCtx = stat match + def statCtx = stat match case _: DefTree | _: ImportOrExport => ctx case _ => ctx.exprContext(stat, exprOwner) - val restCtx = stat match + def restCtx = stat match case stat: Import => ctx.importContext(stat, stat.symbol) case _ => ctx val stat1 = transform(stat)(using statCtx) - if stat1 ne stat then recur(stats, stat1, rest)(using restCtx) - else traverse(rest)(using restCtx) + if stat1 eq stat then + loop(mapped, unchanged, rest) + else + val buf = if mapped == null then new mutable.ListBuffer[Tree] else mapped + var xc = unchanged + while xc ne pending do + buf += xc.head + xc = xc.tail + stat1 match + case Thicket(stats1) => buf ++= stats1 + case _ => buf += stat1 + loop(buf, rest, rest)(using restCtx) case nil => - stats + if mapped == null then unchanged + else mapped.prependToList(unchanged) - traverse(stats) + loop(null, stats, stats) end transformStats - end TreeMapWithPreciseStatContexts /** Map Inlined nodes, NamedArgs, Blocks with no statements and local references to underlying arguments. From 7608bb427ddda38acd4bd5af7b1871d6fecdbd9a Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 28 Jan 2022 14:02:17 +0100 Subject: [PATCH 4/4] Use a single "mapStatements" implementation in three places Use a single "mapStatements" implementation in three TreeMapWithImplicits, MacroTransform, and MegaPhase. --- compiler/src/dotty/tools/dotc/ast/tpd.scala | 36 +++++++++---------- .../tools/dotc/transform/MegaPhase.scala | 36 +------------------ 2 files changed, 19 insertions(+), 53 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/tpd.scala b/compiler/src/dotty/tools/dotc/ast/tpd.scala index 3e63f14e2634..98ea6e0c5c44 100644 --- a/compiler/src/dotty/tools/dotc/ast/tpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/tpd.scala @@ -1151,35 +1151,26 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { case _ => tree1 :: rest1 case nil => nil recur(trees, 0) - end extension - - /** A treemap that generates the same contexts as the original typer for statements. - * This means: - * - statements that are not definitions get the exprOwner as owner - * - imports are reflected in the contexts of subsequent statements - */ - class TreeMapWithPreciseStatContexts(cpy: TreeCopier = tpd.cpy) extends TreeMap(cpy): - /** Transform statements, while maintaining import contexts and expression contexts + /** Transform statements while maintaining import contexts and expression contexts * in the same way as Typer does. The code addresses additional concerns: * - be tail-recursive where possible * - don't re-allocate trees where nothing has changed - * - avoid stack overflows for long statement lists */ - override def transformStats(stats: List[Tree], exprOwner: Symbol)(using Context): List[Tree] = + inline def mapStatements(exprOwner: Symbol, inline op: Tree => Context ?=> Tree)(using Context): List[Tree] = @tailrec def loop(mapped: mutable.ListBuffer[Tree] | Null, unchanged: List[Tree], pending: List[Tree])(using Context): List[Tree] = pending match case stat :: rest => - def statCtx = stat match + val statCtx = stat match case _: DefTree | _: ImportOrExport => ctx case _ => ctx.exprContext(stat, exprOwner) - def restCtx = stat match + val stat1 = op(stat)(using statCtx) + val restCtx = stat match case stat: Import => ctx.importContext(stat, stat.symbol) case _ => ctx - val stat1 = transform(stat)(using statCtx) if stat1 eq stat then - loop(mapped, unchanged, rest) + loop(mapped, unchanged, rest)(using restCtx) else val buf = if mapped == null then new mutable.ListBuffer[Tree] else mapped var xc = unchanged @@ -1194,9 +1185,18 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { if mapped == null then unchanged else mapped.prependToList(unchanged) - loop(null, stats, stats) - end transformStats - end TreeMapWithPreciseStatContexts + loop(null, trees, trees) + end mapStatements + end extension + + /** A treemap that generates the same contexts as the original typer for statements. + * This means: + * - statements that are not definitions get the exprOwner as owner + * - imports are reflected in the contexts of subsequent statements + */ + class TreeMapWithPreciseStatContexts(cpy: TreeCopier = tpd.cpy) extends TreeMap(cpy): + override def transformStats(trees: List[Tree], exprOwner: Symbol)(using Context): List[Tree] = + trees.mapStatements(exprOwner, transform(_)) /** Map Inlined nodes, NamedArgs, Blocks with no statements and local references to underlying arguments. * Also drops Inline and Block with no statements. diff --git a/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala b/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala index a7a19d03e9ff..1d8a2d6b9eee 100644 --- a/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala +++ b/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala @@ -433,43 +433,9 @@ class MegaPhase(val miniPhases: Array[MiniPhase]) extends Phase { transformTree(tree, start).asInstanceOf[T] def transformStats(trees: List[Tree], exprOwner: Symbol, start: Int)(using Context): List[Tree] = - - def transformStat(stat: Tree)(using Context): Tree = stat match { - case _: Import | _: DefTree => transformTree(stat, start) - case Thicket(stats) => cpy.Thicket(stat)(stats.mapConserve(transformStat)) - case _ => transformTree(stat, start)(using ctx.exprContext(stat, exprOwner)) - } - - def restContext(tree: Tree)(using Context): Context = tree match - case imp: Import => ctx.importContext(imp, imp.symbol) - case _ => ctx - - // A slower implementation that avoids deep recursions and stack overflows - def transformSlow(trees: List[Tree])(using Context): List[Tree] = - var curCtx = ctx - flatten(trees.mapConserve { tree => - val tree1 = transformStat(tree)(using curCtx) - curCtx = restContext(tree)(using curCtx) - tree1 - }) - - def recur(trees: List[Tree], count: Int)(using Context): List[Tree] = - if count > MapRecursionLimit then - transformSlow(trees) - else trees match - case tree :: rest => - val tree1 = transformStat(tree) - val rest1 = recur(rest, count + 1)(using restContext(tree)) - if (tree1 eq tree) && (rest1 eq rest) then trees - else tree1 match - case Thicket(elems1) => elems1 ::: rest1 - case _ => tree1 :: rest1 - case nil => nil - val nestedCtx = prepStats(trees, start) - val trees1 = recur(trees, 0)(using nestedCtx) + val trees1 = trees.mapStatements(exprOwner, transformTree(_, start))(using nestedCtx) goStats(trees1, start)(using nestedCtx) - end transformStats def transformUnit(tree: Tree)(using Context): Tree = { val nestedCtx = prepUnit(tree, 0)