From f68bd83af69c5fefbea378f1cfa979302661d9ec Mon Sep 17 00:00:00 2001 From: odersky Date: Sun, 22 Jun 2025 15:44:49 +0200 Subject: [PATCH 1/3] Drop PruneErasedDefs The motivation for the phase was always unclear to me. No tests broke after the change. Piggybacked on the phase was the unrelated "anonymous class in inline method" warning. I moved it in changed form to FirstTransform. It is more efficient now since it does not need an auxiliary tree traversal. --- compiler/src/dotty/tools/dotc/Compiler.scala | 3 +- .../src/dotty/tools/dotc/cc/ccConfig.scala | 2 +- .../tools/dotc/transform/FirstTransform.scala | 7 ++ .../tools/dotc/transform/PostTyper.scala | 46 ++-------- .../dotc/transform/PruneErasedDefs.scala | 88 ------------------- .../tools/dotc/transform/ResolveSuper.scala | 3 +- .../xsbt/CompileProgressSpecification.scala | 2 +- .../macro-late-suspend/test.scoverage.check | 34 +++---- tests/pos/erased-pure.scala | 23 +++++ 9 files changed, 60 insertions(+), 148 deletions(-) delete mode 100644 compiler/src/dotty/tools/dotc/transform/PruneErasedDefs.scala create mode 100644 tests/pos/erased-pure.scala diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala index 6aab7d54d59e..37a59ec4c91d 100644 --- a/compiler/src/dotty/tools/dotc/Compiler.scala +++ b/compiler/src/dotty/tools/dotc/Compiler.scala @@ -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 diff --git a/compiler/src/dotty/tools/dotc/cc/ccConfig.scala b/compiler/src/dotty/tools/dotc/cc/ccConfig.scala index 7836d7fd54a7..4cc2264c12cb 100644 --- a/compiler/src/dotty/tools/dotc/cc/ccConfig.scala +++ b/compiler/src/dotty/tools/dotc/cc/ccConfig.scala @@ -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`) end ccConfig diff --git a/compiler/src/dotty/tools/dotc/transform/FirstTransform.scala b/compiler/src/dotty/tools/dotc/transform/FirstTransform.scala index 8fc9f02c1e38..81cc73c26ed6 100644 --- a/compiler/src/dotty/tools/dotc/transform/FirstTransform.scala +++ b/compiler/src/dotty/tools/dotc/transform/FirstTransform.scala @@ -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" @@ -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 diff --git a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala index ea35d429e3ef..5e2ff2d43283 100644 --- a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala +++ b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala @@ -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)) @@ -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. @@ -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)) @@ -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) @@ -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 diff --git a/compiler/src/dotty/tools/dotc/transform/PruneErasedDefs.scala b/compiler/src/dotty/tools/dotc/transform/PruneErasedDefs.scala deleted file mode 100644 index 47eb70cb46d4..000000000000 --- a/compiler/src/dotty/tools/dotc/transform/PruneErasedDefs.scala +++ /dev/null @@ -1,88 +0,0 @@ -package dotty.tools.dotc -package transform - -import core.* -import Contexts.* -import DenotTransformers.SymTransformer -import Flags.* -import SymDenotations.* -import Symbols.* -import typer.RefChecks -import MegaPhase.MiniPhase -import ast.tpd -import reporting.InlinedAnonClassWarning - -import config.Feature -import Decorators.* -import dotty.tools.dotc.core.Types.MethodType - -/** 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 - * ResolveSuper that inspect class members work correctly. - * 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. - */ -class PruneErasedDefs extends MiniPhase with SymTransformer { thisTransform => - import tpd.* - import PruneErasedDefs.* - - override def phaseName: String = PruneErasedDefs.name - - override def description: String = PruneErasedDefs.description - - override def changesMembers: Boolean = true // makes erased members private - - override def runsAfterGroupsOf: Set[String] = Set(RefChecks.name, ExplicitOuter.name) - - override def transformSym(sym: SymDenotation)(using Context): SymDenotation = - if !sym.isEffectivelyErased || !sym.isTerm || sym.is(Private) || !sym.owner.isClass then sym - else sym.copySymDenotation(initFlags = sym.flags | Private) - - override def transformApply(tree: Apply)(using Context): Tree = - tree.fun.tpe.widen match - case mt: MethodType if mt.hasErasedParams => - cpy.Apply(tree)(tree.fun, tree.args.zip(mt.erasedParams).map((a, e) => if e then trivialErasedTree(a) else a)) - case _ => - tree - - override def transformValDef(tree: ValDef)(using Context): Tree = - checkErasedInExperimental(tree.symbol) - if !tree.symbol.isEffectivelyErased || tree.rhs.isEmpty then tree - else cpy.ValDef(tree)(rhs = trivialErasedTree(tree.rhs)) - - override def transformDefDef(tree: DefDef)(using Context): Tree = - def checkNoInlineAnnoClasses(tree: DefDef)(using Context): Unit = - if tree.symbol.is(Inline) then - new TreeTraverser { - def traverse(tree: Tree)(using Context): Unit = - tree match - case tree: TypeDef if tree.symbol.isAnonymousClass => - report.warning(new InlinedAnonClassWarning(), tree.symbol.sourcePos) - case _ => traverseChildren(tree) - }.traverse(tree) - - checkNoInlineAnnoClasses(tree) - checkErasedInExperimental(tree.symbol) - if !tree.symbol.isEffectivelyErased || tree.rhs.isEmpty then tree - else cpy.DefDef(tree)(rhs = trivialErasedTree(tree.rhs)) - - override def transformTypeDef(tree: TypeDef)(using Context): Tree = - checkErasedInExperimental(tree.symbol) - 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) -} - -object PruneErasedDefs { - import tpd.* - - val name: String = "pruneErasedDefs" - val description: String = "drop erased definitions and simplify erased expressions" - - def trivialErasedTree(tree: Tree)(using Context): Tree = - ref(defn.Compiletime_erasedValue).appliedToType(tree.tpe).withSpan(tree.span) -} diff --git a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala index 67fd233e117a..185b4c0b5eb4 100644 --- a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala +++ b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala @@ -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 diff --git a/sbt-bridge/test/xsbt/CompileProgressSpecification.scala b/sbt-bridge/test/xsbt/CompileProgressSpecification.scala index dc3956ada0db..3fb158e8dd8f 100644 --- a/sbt-bridge/test/xsbt/CompileProgressSpecification.scala +++ b/sbt-bridge/test/xsbt/CompileProgressSpecification.scala @@ -63,7 +63,7 @@ class CompileProgressSpecification { "staging", "splicing", "pickleQuotes", - "MegaPhase{pruneErasedDefs,...,arrayConstructors}", + "MegaPhase{uninitialized,...,arrayConstructors}", "erasure", "constructors", "genBCode" diff --git a/tests/coverage/pos/macro-late-suspend/test.scoverage.check b/tests/coverage/pos/macro-late-suspend/test.scoverage.check index f962705ed2ce..5316c26b38e7 100644 --- a/tests/coverage/pos/macro-late-suspend/test.scoverage.check +++ b/tests/coverage/pos/macro-late-suspend/test.scoverage.check @@ -18,6 +18,23 @@ # - description (can be multi-line) # ' ' sign # ------------------------------------------ +0 +macro-late-suspend/UsesTest.scala +example +UsesTest$package +Object +example.UsesTest$package + +22 +22 +3 + +Literal +true +0 +false + + 1 macro-late-suspend/VisitorMacros.scala example @@ -69,20 +86,3 @@ false false mkVisitorType[Test] -4 -macro-late-suspend/UsesTest.scala -example -UsesTest$package -Object -example.UsesTest$package - -22 -22 -3 - -Literal -true -0 -false - - diff --git a/tests/pos/erased-pure.scala b/tests/pos/erased-pure.scala new file mode 100644 index 000000000000..9d2b54ac02b4 --- /dev/null +++ b/tests/pos/erased-pure.scala @@ -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")))) + + + + + From ac1e0b5d468a0d54ec26dfe4c744de17d9e8e998 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 23 Jun 2025 13:42:04 +0200 Subject: [PATCH 2/3] Fix isPureExpr test We assumed all type applications were pure, which is claerly false. E.g. def fn[T] = println("hi") fn[Int] This triggered a problem in coverage tests which now assumes a use of an erased function is not erased (probably because it now shows up in the coverage info). The problem was fixed by mapping the erased defs to inline defs. I found out the problem with coverage: If an argument is not a pure expression, it gets lifted into a prefix val. And the prefix val is not erased even if the parameter is erased. So that way things escape an erased context. I believe this is a fundamental problem that would also affect lifting due to other reasons (e.g. eta expansion or handling default arguments). We either have to fix the lifting to mainain erased status, or restrict erased arguments to pure expressions that don't need lifting. --- .../src/dotty/tools/dotc/ast/TreeInfo.scala | 16 ++++++++++------ .../tools/dotc/transform/InlineVals.scala | 18 ++++++++++-------- tests/coverage/run/erased/test.scala | 4 ++-- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index 2a9e643aa5b0..0a2c0c850e5d 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -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, _) => diff --git a/compiler/src/dotty/tools/dotc/transform/InlineVals.scala b/compiler/src/dotty/tools/dotc/transform/InlineVals.scala index cff1632ffcd2..c300352a162e 100644 --- a/compiler/src/dotty/tools/dotc/transform/InlineVals.scala +++ b/compiler/src/dotty/tools/dotc/transform/InlineVals.scala @@ -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: @@ -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 diff --git a/tests/coverage/run/erased/test.scala b/tests/coverage/run/erased/test.scala index 6645020cac80..caab36b34066 100644 --- a/tests/coverage/run/erased/test.scala +++ b/tests/coverage/run/erased/test.scala @@ -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 From 1079192a40b05c6e2d66195a960e9f588a289451 Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 24 Jun 2025 16:04:11 +0200 Subject: [PATCH 3/3] Relax double definition check for inline methods If one of the methods is an inline method that is not retained, no need to report a double definition. --- .../dotty/tools/dotc/transform/ElimErasedValueType.scala | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/ElimErasedValueType.scala b/compiler/src/dotty/tools/dotc/transform/ElimErasedValueType.scala index 2deb50956537..e97bf1ed6dd1 100644 --- a/compiler/src/dotty/tools/dotc/transform/ElimErasedValueType.scala +++ b/compiler/src/dotty/tools/dotc/transform/ElimErasedValueType.scala @@ -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) {