Skip to content

Refactorings and fixes to erased definition handling #23404

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 3 commits into from
Jun 24, 2025
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
3 changes: 1 addition & 2 deletions compiler/src/dotty/tools/dotc/Compiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ class Compiler {
new ExplicitSelf, // Make references to non-trivial self types explicit as casts
new StringInterpolatorOpt, // Optimizes raw and s and f string interpolators by rewriting them to string concatenations or formats
new DropBreaks) :: // Optimize local Break throws by rewriting them
List(new PruneErasedDefs, // Drop erased definitions from scopes and simplify erased expressions
new UninitializedDefs, // Replaces `compiletime.uninitialized` by `_`
List(new UninitializedDefs, // Replaces `compiletime.uninitialized` by `_`
new InlinePatterns, // Remove placeholders of inlined patterns
new VCInlineMethods, // Inlines calls to value class methods
new SeqLiterals, // Express vararg arguments as arrays
Expand Down
16 changes: 10 additions & 6 deletions compiler/src/dotty/tools/dotc/ast/TreeInfo.scala
Original file line number Diff line number Diff line change
Expand Up @@ -588,14 +588,18 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
case New(_) | Closure(_, _, _) =>
Pure
case TypeApply(fn, _) =>
if (fn.symbol.is(Erased) || fn.symbol == defn.QuotedTypeModule_of || fn.symbol == defn.Predef_classOf) Pure else exprPurity(fn)
if tree.tpe.isInstanceOf[MethodOrPoly] then exprPurity(fn)
else if fn.symbol == defn.QuotedTypeModule_of || fn.symbol == defn.Predef_classOf then Pure
else if fn.symbol == defn.Compiletime_erasedValue && tree.tpe.dealias.isInstanceOf[ConstantType] then Pure
else Impure
case Apply(fn, args) =>
if isPureApply(tree, fn) then
minOf(exprPurity(fn), args.map(exprPurity)) `min` Pure
else if fn.symbol.is(Erased) then
Pure
val factorPurity = minOf(exprPurity(fn), args.map(exprPurity))
if tree.tpe.isInstanceOf[MethodOrPoly] then // no evaluation
factorPurity `min` Pure
else if isPureApply(tree, fn) then
factorPurity `min` Pure
else if fn.symbol.isStableMember /* && fn.symbol.is(Lazy) */ then
minOf(exprPurity(fn), args.map(exprPurity)) `min` Idempotent
factorPurity `min` Idempotent
else
Impure
case Typed(expr, _) =>
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/cc/ccConfig.scala
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,6 @@ object ccConfig:

/** Not used currently. Handy for trying out new features */
def newScheme(using ctx: Context): Boolean =
ctx.settings.XdropComments.value
Feature.sourceVersion.stable.isAtLeast(SourceVersion.`3.7`)
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to be a random change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, it's not needed elsewhere though.


end ccConfig
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,15 @@ class ElimErasedValueType extends MiniPhase with InfoTransformer { thisPhase =>
(sym1.owner.derivesFrom(defn.PolyFunctionClass) ||
sym2.owner.derivesFrom(defn.PolyFunctionClass))

def oneErasedInline =
sym1.isInlineMethod && !sym1.isRetainedInlineMethod
|| sym2.isInlineMethod && !sym2.isRetainedInlineMethod

// super-accessors start as private, and their expanded name can clash after
// erasure. TODO: Verify that this is OK.
def bothSuperAccessors = sym1.name.is(SuperAccessorName) && sym2.name.is(SuperAccessorName)
if (sym1.name != sym2.name && !bothSuperAccessors ||
!info1.matchesLoosely(info2) && !bothPolyApply)
if (sym1.name != sym2.name && !bothSuperAccessors
|| !info1.matchesLoosely(info2) && !bothPolyApply && !oneErasedInline)
report.error(DoubleDefinition(sym1, sym2, root), root.srcPos)
}
while (opc.hasNext) {
Expand Down
7 changes: 7 additions & 0 deletions compiler/src/dotty/tools/dotc/transform/FirstTransform.scala
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import StdNames.*
import config.Feature
import inlines.Inlines.inInlineMethod
import util.Property
import inlines.Inlines
import reporting.InlinedAnonClassWarning

object FirstTransform {
val name: String = "firstTransform"
Expand Down Expand Up @@ -207,6 +209,11 @@ class FirstTransform extends MiniPhase with SymTransformer { thisPhase =>
case _ => tree
}

override def transformTypeDef(tree: TypeDef)(using Context): Tree =
if tree.symbol.isAnonymousClass && Inlines.inInlineMethod then
report.warning(InlinedAnonClassWarning(), tree.symbol.sourcePos)
tree

/** Perform one of the following simplification if applicable:
*
* true && y ==> y
Expand Down
18 changes: 10 additions & 8 deletions compiler/src/dotty/tools/dotc/transform/InlineVals.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@ package dotty.tools
package dotc
package transform

import dotty.tools.dotc.core.Contexts.*
import dotty.tools.dotc.core.Decorators.*
import dotty.tools.dotc.core.Symbols.*
import dotty.tools.dotc.core.Flags.*
import dotty.tools.dotc.core.Types.*
import dotty.tools.dotc.transform.MegaPhase.MiniPhase
import dotty.tools.dotc.inlines.Inlines
import core.*
import Contexts.*, Decorators.*, Symbols.*, Flags.*, Types.*
import MegaPhase.MiniPhase
import inlines.Inlines
import ast.tpd


/** Check that `tree.rhs` can be right hand-side of an `inline` value definition. */
class InlineVals extends MiniPhase:
Expand Down Expand Up @@ -38,7 +37,10 @@ class InlineVals extends MiniPhase:
tpt.tpe.widenTermRefExpr.dealiasKeepOpaques.normalized match
case tp: ConstantType =>
if !isPureExpr(rhs) then
def details = if enclosingInlineds.isEmpty then "" else i"but was: $rhs"
def details =
if enclosingInlineds.nonEmpty || rhs.isInstanceOf[tpd.Inlined]
then i" but was: $rhs"
else ""
report.error(em"inline value must be pure$details", rhs.srcPos)
case tp =>
if tp.typeSymbol.is(Opaque) then
Expand Down
46 changes: 9 additions & 37 deletions compiler/src/dotty/tools/dotc/transform/PostTyper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -335,15 +335,6 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
}
}

private object dropInlines extends TreeMap {
override def transform(tree: Tree)(using Context): Tree = tree match {
case tree @ Inlined(call, _, expansion) =>
val newExpansion = PruneErasedDefs.trivialErasedTree(tree)
cpy.Inlined(tree)(call, Nil, newExpansion)
case _ => super.transform(tree)
}
}

def checkUsableAsValue(tree: Tree)(using Context): Tree =
def unusable(msg: Symbol => Message) =
errorTree(tree, msg(tree.symbol))
Expand Down Expand Up @@ -414,26 +405,13 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
checkUsableAsValue(tree) match
case tree1: Select => transformSelect(tree1, Nil)
case tree1 => tree1
case tree: Apply =>
val methType = tree.fun.tpe.widen.asInstanceOf[MethodType]
val app =
if (methType.hasErasedParams)
tpd.cpy.Apply(tree)(
tree.fun,
tree.args.zip(methType.erasedParams).map((arg, isErased) =>
if !isErased then arg
else
if methType.isResultDependent then
Checking.checkRealizable(arg.tpe, arg.srcPos, "erased argument")
if (methType.isImplicitMethod && arg.span.isSynthetic)
arg match
case _: RefTree | _: Apply | _: TypeApply if arg.symbol.is(Erased) =>
dropInlines.transform(arg)
case _ =>
PruneErasedDefs.trivialErasedTree(arg)
else dropInlines.transform(arg)))
else
tree
case app: Apply =>
val methType = app.fun.tpe.widen.asInstanceOf[MethodType]
if (methType.hasErasedParams)
for (arg, isErased) <- app.args.lazyZip(methType.erasedParams) do
if isErased then
if methType.isResultDependent then
Checking.checkRealizable(arg.tpe, arg.srcPos, "erased argument")
def app1 =
// reverse order of transforming args and fun. This way, we get a chance to see other
// well-formedness errors before reporting errors in possible inferred type args of fun.
Expand Down Expand Up @@ -499,7 +477,7 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
registerIfHasMacroAnnotations(tree)
checkErasedDef(tree)
Checking.checkPolyFunctionType(tree.tpt)
val tree1 = cpy.ValDef(tree)(tpt = makeOverrideTypeDeclared(tree.symbol, tree.tpt), rhs = normalizeErasedRhs(tree.rhs, tree.symbol))
val tree1 = cpy.ValDef(tree)(tpt = makeOverrideTypeDeclared(tree.symbol, tree.tpt))
if tree1.removeAttachment(desugar.UntupledParam).isDefined then
checkStableSelection(tree.rhs)
processValOrDefDef(super.transform(tree1))
Expand All @@ -508,7 +486,7 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
checkErasedDef(tree)
Checking.checkPolyFunctionType(tree.tpt)
annotateContextResults(tree)
val tree1 = cpy.DefDef(tree)(tpt = makeOverrideTypeDeclared(tree.symbol, tree.tpt), rhs = normalizeErasedRhs(tree.rhs, tree.symbol))
val tree1 = cpy.DefDef(tree)(tpt = makeOverrideTypeDeclared(tree.symbol, tree.tpt))
processValOrDefDef(superAcc.wrapDefDef(tree1)(super.transform(tree1).asInstanceOf[DefDef]))
case tree: TypeDef =>
registerIfHasMacroAnnotations(tree)
Expand Down Expand Up @@ -632,12 +610,6 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
Checking.checkAndAdaptExperimentalImports(trees)
super.transformStats(trees, exprOwner, wrapResult)

/** Transforms the rhs tree into a its default tree if it is in an `erased` val/def.
* Performed to shrink the tree that is known to be erased later.
*/
private def normalizeErasedRhs(rhs: Tree, sym: Symbol)(using Context) =
if (sym.isEffectivelyErased) dropInlines.transform(rhs) else rhs

private def registerNeedsInlining(tree: Tree)(using Context): Unit =
if tree.symbol.is(Inline) && !Inlines.inInlineMethod && !ctx.mode.is(Mode.NoInline) then
ctx.compilationUnit.needsInlining = true
Expand Down
88 changes: 0 additions & 88 deletions compiler/src/dotty/tools/dotc/transform/PruneErasedDefs.scala

This file was deleted.

3 changes: 1 addition & 2 deletions compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ class ResolveSuper extends MiniPhase with IdentityDenotTransformer { thisPhase =

override def description: String = ResolveSuper.description

override def runsAfter: Set[String] = Set(ElimByName.name, // verified empirically, need to figure out what the reason is.
PruneErasedDefs.name) // Erased decls make `isCurrent` work incorrectly
override def runsAfter: Set[String] = Set(ElimByName.name) // verified empirically, need to figure out what the reason is.

override def changesMembers: Boolean = true // the phase adds super accessors

Expand Down
2 changes: 1 addition & 1 deletion sbt-bridge/test/xsbt/CompileProgressSpecification.scala
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class CompileProgressSpecification {
"staging",
"splicing",
"pickleQuotes",
"MegaPhase{pruneErasedDefs,...,arrayConstructors}",
"MegaPhase{uninitialized,...,arrayConstructors}",
"erasure",
"constructors",
"genBCode"
Expand Down
34 changes: 17 additions & 17 deletions tests/coverage/pos/macro-late-suspend/test.scoverage.check
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,23 @@
# - description (can be multi-line)
# ' ' sign
# ------------------------------------------
0
macro-late-suspend/UsesTest.scala
example
UsesTest$package
Object
example.UsesTest$package
<init>
22
22
3
<none>
Literal
true
0
false


Comment on lines +21 to +37
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what this test does. Is it important that the output changes?

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 am not sure. It might have to do with the fact that we don't delete code in PruneErasedDefs anymore. But if anything this hshould be more precise now.

1
macro-late-suspend/VisitorMacros.scala
example
Expand Down Expand Up @@ -69,20 +86,3 @@ false
false
mkVisitorType[Test]

4
macro-late-suspend/UsesTest.scala
example
UsesTest$package
Object
example.UsesTest$package
<init>
22
22
3
<none>
Literal
true
0
false


4 changes: 2 additions & 2 deletions tests/coverage/run/erased/test.scala
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import scala.language.experimental.erasedDefinitions

erased def parameterless: String = "y"
inline def parameterless: String = "y"

erased def e(erased x: String): String = "x"
inline def e(erased x: String): String = "x"
def foo(erased a: String)(b: String): String =
println(s"foo(a)($b)")
b
Expand Down
23 changes: 23 additions & 0 deletions tests/pos/erased-pure.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import language.experimental.erasedDefinitions

inline def id[T](x: T) = x

class C()

def foo[T](erased x: T): Unit = ()

class Pair[A, B](x: A, y: B)


def Test =
foo(C())
foo(id(C()))
foo(Pair(C(), C()))
foo(Pair(C(), 22))
foo(Pair(C(), "hello" + "world"))
foo(id(Pair(id(C()), id("hello" + "world"))))





Loading