Keep language imports around longer #14364
Conversation
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 scala#14346
|
test performance please |
|
performance test scheduled: 4 job(s) in queue, 1 running. |
|
Performance test finished successfully: Visit https://dotty-bench.epfl.ch/14364/ to see the changes. Benchmarks is based on merging with master (8ae2962) |
|
test performance please |
|
performance test scheduled: 2 job(s) in queue, 1 running. |
| @@ -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 | |||
There was a problem hiding this comment.
I thought about making a separate miniphase for this but it's probably not worth the overhead of boilerplate. If we want to move the dropping of imports, we can easily split it out into a separate miniphase later.
There was a problem hiding this comment.
Yes, that was my thinking also.
|
Performance test finished successfully: Visit https://dotty-bench.epfl.ch/14364/ to see the changes. Benchmarks is based on merging with master (8ae2962) |
| 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 |
There was a problem hiding this comment.
Could we add this for MacroTransform as well? I need the import information in PostTyper to fix #14319.
There was a problem hiding this comment.
Sure, let's do that.
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.
Use a single "mapStatements" implementation in three TreeMapWithImplicits, MacroTransform, and MegaPhase.
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 0 running. |
|
Performance test finished successfully: Visit https://dotty-bench.epfl.ch/14364/ to see the changes. Benchmarks is based on merging with master (f955f8b) |
Some phases might need to know what language imports are active. Concretely, under
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.
Update: Now, MacroTransform has the same changes, and in fact all of MegaPhase, MacroTransform and TreeMapWithImplicits use the same inline method for treating statement lists.
Fixes #14346