Skip to content

Commit c5a3b5c

Browse files
smartersmarter
authored
Merge pull request #8428 from dotty-staging/fix-#7926
Fix #7926: Redo parameter aliasing optimization
2 parents 5c6098f + a924104 commit c5a3b5c

29 files changed

+215
-123
lines changed

‎community-build/community-projects/stdLib213

‎compiler/src/dotty/tools/dotc/Compiler.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ class Compiler {
9090
new AugmentScala2Traits, // Augments Scala2 traits with additional members needed for mixin composition.
9191
new ResolveSuper, // Implement super accessors
9292
new FunctionXXLForwarders, // Add forwarders for FunctionXXL apply method
93+
new ParamForwarding, // Add forwarders for aliases of superclass parameters
9394
new TupleOptimizations, // Optimize generic operations on tuples
9495
new ArrayConstructors) :: // Intercept creation of (non-generic) arrays and intrinsify.
9596
List(new Erasure) :: // Rewrite types to JVM model, erasing all type parameters, abstract types and refinements.

‎compiler/src/dotty/tools/dotc/core/Annotations.scala

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,6 @@ object Annotations {
145145
def deferredResolve(atp: Type, args: List[Tree])(implicit ctx: Context): Annotation =
146146
deferred(atp.classSymbol)(resolveConstructor(atp, args))
147147

148-
def makeAlias(sym: TermSymbol)(implicit ctx: Context): Annotation =
149-
apply(defn.AliasAnnot, List(
150-
ref(TermRef(sym.owner.thisType, sym.name, sym))))
151-
152148
/** Extractor for child annotations */
153149
object Child {
154150

‎compiler/src/dotty/tools/dotc/core/Definitions.scala

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -780,7 +780,6 @@ class Definitions {
780780
@tu lazy val RefiningAnnotationClass: ClassSymbol = ctx.requiredClass("scala.annotation.RefiningAnnotation")
781781

782782
// Annotation classes
783-
@tu lazy val AliasAnnot: ClassSymbol = ctx.requiredClass("scala.annotation.internal.Alias")
784783
@tu lazy val AnnotationDefaultAnnot: ClassSymbol = ctx.requiredClass("scala.annotation.internal.AnnotationDefault")
785784
@tu lazy val BodyAnnot: ClassSymbol = ctx.requiredClass("scala.annotation.internal.Body")
786785
@tu lazy val ChildAnnot: ClassSymbol = ctx.requiredClass("scala.annotation.internal.Child")

‎compiler/src/dotty/tools/dotc/core/Flags.scala

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ object Flags {
296296
val (_, CaseAccessor @ _, _) = newFlags(25, "<caseaccessor>")
297297

298298
/** A Scala 2x super accessor / an unpickled Scala 2.x class */
299-
val (SuperAccessorOrScala2x @ _, Scala2SuperAccessor @ _, Scala2x @ _) = newFlags(26, "<superaccessor>", "<scala-2.x>")
299+
val (SuperParamAliasOrScala2x @ _, SuperParamAlias @ _, Scala2x @ _) = newFlags(26, "<super-param-alias>", "<scala-2.x>")
300300

301301
/** A method that has default params */
302302
val (_, DefaultParameterized @ _, _) = newFlags(27, "<defaultparam>")
@@ -371,8 +371,9 @@ object Flags {
371371
/** Symbol is a self name */
372372
val (_, SelfName @ _, _) = newFlags(54, "<selfname>")
373373

374-
/** An existentially bound symbol (Scala 2.x only) */
375-
val (Scala2ExistentialCommon @ _, _, Scala2Existential @ _) = newFlags(55, "<existential>")
374+
/** A Scala 2 superaccessor (only needed during Scala2Unpickling) /
375+
* an existentially bound symbol (Scala 2.x only) */
376+
val (Scala2SpecialFlags @ _, Scala2SuperAccessor @ _, Scala2Existential @ _) = newFlags(55, "<existential>")
376377

377378
/** Children were queried on this class */
378379
val (_, _, ChildrenQueried @ _) = newFlags(56, "<children-queried>")
@@ -439,10 +440,10 @@ object Flags {
439440
val FromStartFlags: FlagSet = commonFlags(
440441
Module, Package, Deferred, Method, Case,
441442
HigherKinded, Param, ParamAccessor,
442-
Scala2ExistentialCommon, MutableOrOpen, Opaque, Touched, JavaStatic,
443+
Scala2SpecialFlags, MutableOrOpen, Opaque, Touched, JavaStatic,
443444
OuterOrCovariant, LabelOrContravariant, CaseAccessor,
444445
Extension, NonMember, Implicit, Given, Permanent, Synthetic,
445-
SuperAccessorOrScala2x, Inline, Macro)
446+
SuperParamAliasOrScala2x, Inline, Macro)
446447

447448
/** Flags that are not (re)set when completing the denotation, or, if symbol is
448449
* a top-level class or object, when completing the denotation once the class
@@ -551,6 +552,7 @@ object Flags {
551552
val JavaProtected: FlagSet = JavaDefined | Protected
552553
val MethodOrLazy: FlagSet = Lazy | Method
553554
val MutableOrLazy: FlagSet = Lazy | Mutable
555+
val MethodOrLazyOrMutable: FlagSet = Lazy | Method | Mutable
554556
val LiftedMethod: FlagSet = Lifted | Method
555557
val LocalParam: FlagSet = Local | Param
556558
val LocalParamAccessor: FlagSet = Local | ParamAccessor | Private
@@ -561,7 +563,7 @@ object Flags {
561563
val PrivateMethod: FlagSet = Method | Private
562564
val NoInitsInterface: FlagSet = NoInits | PureInterface
563565
val NoInitsTrait: FlagSet = NoInits | Trait // A trait that does not need to be initialized
564-
val ValidForeverFlags: FlagSet = Package | Permanent | Scala2ExistentialCommon
566+
val ValidForeverFlags: FlagSet = Package | Permanent | Scala2SpecialFlags
565567
val TermParamOrAccessor: FlagSet = Param | ParamAccessor
566568
val PrivateParamAccessor: FlagSet = ParamAccessor | Private
567569
val PrivateOrSynthetic: FlagSet = Private | Synthetic

‎compiler/src/dotty/tools/dotc/core/NameKinds.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,12 +354,12 @@ object NameKinds {
354354
val InlineAccessorName: PrefixNameKind = new PrefixNameKind(INLINEACCESSOR, "inline$")
355355

356356
val AvoidClashName: SuffixNameKind = new SuffixNameKind(AVOIDCLASH, "$_avoid_name_clash_$")
357-
val CacheName = new SuffixNameKind(CACHE, "$_cache")
358357
val DirectMethodName: SuffixNameKind = new SuffixNameKind(DIRECT, "$direct") { override def definesNewName = true }
359358
val FieldName: SuffixNameKind = new SuffixNameKind(FIELD, "$$local") {
360359
override def mkString(underlying: TermName, info: ThisInfo) = underlying.toString
361360
}
362361
val ExtMethName: SuffixNameKind = new SuffixNameKind(EXTMETH, "$extension")
362+
val ParamAccessorName: SuffixNameKind = new SuffixNameKind(PARAMACC, "$accessor")
363363
val ModuleClassName: SuffixNameKind = new SuffixNameKind(OBJECTCLASS, "$", optInfoString = "ModuleClass")
364364
val ImplMethName: SuffixNameKind = new SuffixNameKind(IMPLMETH, "$")
365365
val AdaptedClosureName: SuffixNameKind = new SuffixNameKind(ADAPTEDCLOSURE, "$adapted") { override def definesNewName = true }

‎compiler/src/dotty/tools/dotc/core/NameTags.scala

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ object NameTags extends TastyFormat.NameTags {
3434
final val IMPLMETH = 32 // Used to define methods in implementation classes
3535
// (can probably be removed).
3636

37-
final val CACHE = 33 // Used as a cache for the rhs of an alias implicit.
37+
final val PARAMACC = 33 // Used for a private parameter alias
3838

3939
def nameTagToString(tag: Int): String = tag match {
4040
case UTF8 => "UTF8"
@@ -55,6 +55,8 @@ object NameTags extends TastyFormat.NameTags {
5555
case DIRECT => "DIRECT"
5656
case FIELD => "FIELD"
5757
case EXTMETH => "EXTMETH"
58+
case IMPLMETH => "IMPLMETH"
59+
case PARAMACC => "PARAMACC"
5860
case ADAPTEDCLOSURE => "ADAPTEDCLOSURE"
5961
case OBJECTCLASS => "OBJECTCLASS"
6062

‎compiler/src/dotty/tools/dotc/core/SymDenotations.scala

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1485,6 +1485,11 @@ object SymDenotations {
14851485
override def transformAfter(phase: DenotTransformer, f: SymDenotation => SymDenotation)(implicit ctx: Context): Unit =
14861486
super.transformAfter(phase, f)
14871487

1488+
/** Set flag `flags` in current phase and in all phases that follow */
1489+
def setFlagFrom(phase: DenotTransformer, flags: FlagSet)(using Context): Unit =
1490+
setFlag(flags)
1491+
transformAfter(phase, sd => { sd.setFlag(flags); sd })
1492+
14881493
/** If denotation is private, remove the Private flag and expand the name if necessary */
14891494
def ensureNotPrivate(implicit ctx: Context): SymDenotation =
14901495
if (is(Private))

‎compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -682,6 +682,7 @@ class TreePickler(pickler: TastyPickler) {
682682
if (flags.is(StableRealizable)) writeModTag(STABLE)
683683
if (flags.is(Extension)) writeModTag(EXTENSION)
684684
if (flags.is(ParamAccessor)) writeModTag(PARAMsetter)
685+
if (flags.is(SuperParamAlias)) writeModTag(PARAMalias)
685686
if (flags.is(Exported)) writeModTag(EXPORTED)
686687
assert(!(flags.is(Label)))
687688
}

‎compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,7 @@ class TreeUnpickler(reader: TastyReader,
644644
case EXTENSION => addFlag(Extension)
645645
case GIVEN => addFlag(Given)
646646
case PARAMsetter => addFlag(ParamAccessor)
647+
case PARAMalias => addFlag(SuperParamAlias)
647648
case EXPORTED => addFlag(Exported)
648649
case OPEN => addFlag(Open)
649650
case PRIVATEqualified =>

‎compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -598,7 +598,7 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas
598598
denot.info matches denot.owner.thisType.memberInfo(alt)
599599
}
600600
val alias = readDisambiguatedSymbolRef(disambiguate).asTerm
601-
denot.addAnnotation(Annotation.makeAlias(alias))
601+
if alias.name == denot.name then denot.setFlag(SuperParamAlias)
602602
}
603603
}
604604
// println(s"unpickled ${denot.debugString}, info = ${denot.info}") !!! DEBUG

‎compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ class AugmentScala2Traits extends MiniPhase with IdentityDenotTransformer { this
6464
|| sym.isSuperAccessor) // scala2 superaccessors are pickled as private, but are compiled as public expanded
6565
sym.ensureNotPrivate.installAfter(thisPhase)
6666
}
67-
mixin.setFlag(Scala2xPartiallyAugmented)
68-
mixin.transformAfter(thisPhase, d => { d.setFlag(Scala2xPartiallyAugmented); d })
67+
mixin.setFlagFrom(thisPhase, Scala2xPartiallyAugmented)
6968
}
7069
}

‎compiler/src/dotty/tools/dotc/transform/CacheAliasImplicits.scala

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import core.Contexts._
88
import core.Types._
99
import core.Flags._
1010
import core.StdNames.nme
11-
import core.NameKinds.CacheName
1211
import core.Constants.Constant
1312
import core.Decorators._
1413
import core.TypeErasure.erasure

‎compiler/src/dotty/tools/dotc/transform/ParamForwarding.scala

Lines changed: 65 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,88 +1,82 @@
1-
package dotty.tools.dotc
1+
package dotty.tools
2+
package dotc
23
package transform
34

45
import core._
56
import ast.Trees._
67
import Contexts._, Types._, Symbols._, Flags._, TypeUtils._, DenotTransformers._, StdNames._
78
import Decorators._
9+
import MegaPhase._
10+
import NameKinds.ParamAccessorName
811
import config.Printers.typr
912

10-
/** For all parameter accessors
13+
/** For all private parameter accessors
1114
*
12-
* val x: T = ...
15+
* private val x: T = ...
1316
*
14-
* if
15-
* (1) x is forwarded in the supercall to a parameter that's also named `x`
16-
* (2) the superclass parameter accessor for `x` is accessible from the current class
17-
* change the accessor to
17+
* If there is a chain of parameter accessors starting with `x` such that
18+
* (1) The last parameter accessor in the chain is a field that's accessible
19+
* from the current class, and
20+
* (2) each preceding parameter is forwarded in the supercall of its class
21+
* to a parameter that's also named `x`
22+
* then change the accessor to
1823
*
19-
* def x: T = super.x.asInstanceOf[T]
24+
* private def x$accessor: T = super.x'.asInstanceOf[T]
25+
*
26+
* where x' is a reference to the final parameter in the chain.
27+
* Property (1) is established by the @see forwardParamAccessors method in PostTyper.
28+
*
29+
* The reason for renaming `x` to `x$accessor` is that private methods in the JVM
30+
* cannot override public ones.
2031
*
21-
* Do the same also if there are intermediate inaccessible parameter accessor forwarders.
2232
* The aim of this transformation is to avoid redundant parameter accessor fields.
2333
*/
24-
class ParamForwarding(thisPhase: DenotTransformer) {
34+
class ParamForwarding extends MiniPhase with IdentityDenotTransformer:
2535
import ast.tpd._
2636

27-
def forwardParamAccessors(impl: Template)(implicit ctx: Context): Template = {
28-
def fwd(stats: List[Tree])(implicit ctx: Context): List[Tree] = {
29-
val (superArgs, superParamNames) = impl.parents match {
30-
case superCall @ Apply(fn, args) :: _ =>
31-
fn.tpe.widen match {
32-
case MethodType(paramNames) => (args, paramNames)
33-
case _ => (Nil, Nil)
34-
}
35-
case _ => (Nil, Nil)
36-
}
37-
def inheritedAccessor(sym: Symbol): Symbol = {
38-
/**
39-
* Dmitry: having it have the same name is needed to maintain correctness in presence of subclassing
40-
* if you would use parent param-name `a` to implement param-field `b`
41-
* overriding field `b` will actually override field `a`, that is wrong!
42-
*
43-
* class A(val s: Int);
44-
* class B(val b: Int) extends A(b)
45-
* class C extends A(2) {
46-
* def s = 3
47-
* assert(this.b == 2)
48-
* }
49-
*/
50-
val candidate = sym.owner.asClass.superClass
51-
.info.decl(sym.name).suchThat(_.is(ParamAccessor, butNot = Mutable)).symbol
52-
if (candidate.isAccessibleFrom(currentClass.thisType, superAccess = true)) candidate
53-
else if (candidate.exists) inheritedAccessor(candidate)
54-
else NoSymbol
55-
}
56-
def forwardParamAccessor(stat: Tree): Tree = {
57-
stat match {
58-
case stat: ValDef =>
59-
val sym = stat.symbol.asTerm
60-
if (sym.is(ParamAccessor, butNot = Mutable) && !sym.info.isInstanceOf[ExprType]) {
61-
// ElimByName gets confused with methods returning an ExprType,
62-
// so avoid param forwarding if parameter is by name. See i1766.scala
63-
val idx = superArgs.indexWhere(_.symbol == sym)
64-
if (idx >= 0 && superParamNames(idx) == stat.name) { // supercall to like-named parameter
65-
val alias = inheritedAccessor(sym)
66-
if (alias.exists) {
67-
def forwarder(implicit ctx: Context) = {
68-
sym.copySymDenotation(initFlags = sym.flags | Method | StableRealizable, info = sym.info.ensureMethodic)
69-
.installAfter(thisPhase)
70-
val superAcc =
71-
Super(This(currentClass), tpnme.EMPTY, inConstrCall = false).select(alias)
72-
typr.println(i"adding param forwarder $superAcc")
73-
DefDef(sym, superAcc.ensureConforms(sym.info.widen)).withSpan(stat.span)
74-
}
75-
return forwarder(ctx.withPhase(thisPhase.next))
76-
}
77-
}
78-
}
79-
case _ =>
80-
}
81-
stat
82-
}
83-
stats map forwardParamAccessor
84-
}
37+
private def thisPhase: ParamForwarding = this
38+
39+
val phaseName: String = "paramForwarding"
40+
41+
def transformIfParamAlias(mdef: ValOrDefDef)(using ctx: Context): Tree =
42+
43+
def inheritedAccessor(sym: Symbol)(using Context): Symbol =
44+
val candidate = sym.owner.asClass.superClass
45+
.info.decl(sym.name).suchThat(_.is(ParamAccessor, butNot = Mutable))
46+
.symbol
47+
if !candidate.is(Private) // candidate might be private and accessible if it is in an outer class
48+
&& candidate.isAccessibleFrom(currentClass.thisType, superAccess = true)
49+
then
50+
candidate
51+
else if candidate.is(SuperParamAlias) then
52+
inheritedAccessor(candidate)
53+
else
54+
NoSymbol
55+
56+
val sym = mdef.symbol.asTerm
57+
if sym.is(SuperParamAlias) then
58+
assert(sym.is(ParamAccessor, butNot = Mutable))
59+
val alias = inheritedAccessor(sym)(using ctx.withPhase(thisPhase))
60+
if alias.exists then
61+
sym.copySymDenotation(
62+
name = ParamAccessorName(sym.name),
63+
initFlags = sym.flags | Method | StableRealizable,
64+
info = sym.info.ensureMethodic
65+
).installAfter(thisPhase)
66+
val superAcc =
67+
Super(This(currentClass), tpnme.EMPTY, inConstrCall = false)
68+
.withSpan(mdef.span)
69+
.select(alias)
70+
.ensureApplied
71+
.ensureConforms(sym.info.finalResultType)
72+
ctx.log(i"adding param forwarder $superAcc")
73+
DefDef(sym, superAcc)
74+
else mdef
75+
else mdef
76+
end transformIfParamAlias
77+
78+
override def transformValDef(mdef: ValDef)(using Context): Tree =
79+
transformIfParamAlias(mdef)
8580

86-
cpy.Template(impl)(body = fwd(impl.body)(ctx.withPhase(thisPhase)))
87-
}
88-
}
81+
override def transformDefDef(mdef: DefDef)(using Context): Tree =
82+
transformIfParamAlias(mdef)

‎compiler/src/dotty/tools/dotc/transform/PostTyper.scala

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import Types._, Contexts._, Names._, Flags._, DenotTransformers._, Phases._
1111
import SymDenotations._, StdNames._, Annotations._, Trees._, Scopes._
1212
import Decorators._
1313
import Symbols._, SymUtils._
14+
import config.Printers.typr
1415
import reporting.diagnostic.messages._
1516

1617
object PostTyper {
@@ -24,7 +25,7 @@ object PostTyper {
2425
* (2) Convert parameter fields that have the same name as a corresponding
2526
* public parameter field in a superclass to a forwarder to the superclass
2627
* field (corresponding = super class field is initialized with subclass field)
27-
* (@see ForwardParamAccessors)
28+
* @see forwardParamAccessors.
2829
*
2930
* (3) Add synthetic members (@see SyntheticMembers)
3031
*
@@ -72,7 +73,6 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase
7273
new PostTyperTransformer
7374

7475
val superAcc: SuperAccessors = new SuperAccessors(thisPhase)
75-
val paramFwd: ParamForwarding = new ParamForwarding(thisPhase)
7676
val synthMbr: SyntheticMembers = new SyntheticMembers(thisPhase)
7777

7878
private def newPart(tree: Tree): Option[New] = methPart(tree) match {
@@ -98,6 +98,30 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase
9898

9999
def isCheckable(t: New): Boolean = !inJavaAnnot && !noCheckNews.contains(t)
100100

101+
/** Mark parameter accessors that are aliases of like-named parameters
102+
* in their superclass with SuperParamAlias.
103+
* This info is used in phase ParamForwarding
104+
*/
105+
private def forwardParamAccessors(impl: Template)(using Context): Unit = impl.parents match
106+
case superCall @ Apply(fn, superArgs) :: _ if superArgs.nonEmpty =>
107+
fn.tpe.widen match
108+
case MethodType(superParamNames) =>
109+
for case stat: ValDef <- impl.body do
110+
val sym = stat.symbol
111+
if sym.isAllOf(PrivateParamAccessor, butNot = Mutable)
112+
&& !sym.info.isInstanceOf[ExprType] // val-parameters cannot be call-by name, so no need to try to forward to them
113+
then
114+
val idx = superArgs.indexWhere(_.symbol == sym)
115+
if idx >= 0 && superParamNames(idx) == stat.name then
116+
// Supercall to like-named parameter.
117+
// Having it have the same name is needed to maintain correctness in presence of subclassing
118+
// if you would use parent param-name `a` to implement param-field `b`
119+
// overriding field `b` will actually override field `a`, that is wrong!
120+
typr.println(i"super alias: ${sym.showLocated}")
121+
sym.setFlagFrom(thisPhase, SuperParamAlias)
122+
case _ =>
123+
case _ =>
124+
101125
private def transformAnnot(annot: Tree)(implicit ctx: Context): Tree = {
102126
val saved = inJavaAnnot
103127
inJavaAnnot = annot.symbol.is(JavaDefined)
@@ -227,11 +251,11 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase
227251
val pos = call.sourcePos
228252
val callTrace = Inliner.inlineCallTrace(call.symbol, pos)(ctx.withSource(pos.source))
229253
cpy.Inlined(tree)(callTrace, transformSub(bindings), transform(expansion)(inlineContext(call)))
230-
case tree: Template =>
231-
withNoCheckNews(tree.parents.flatMap(newPart)) {
232-
val templ1 = paramFwd.forwardParamAccessors(tree)
254+
case templ: Template =>
255+
withNoCheckNews(templ.parents.flatMap(newPart)) {
256+
forwardParamAccessors(templ)
233257
synthMbr.addSyntheticMembers(
234-
superAcc.wrapTemplate(templ1)(
258+
superAcc.wrapTemplate(templ)(
235259
super.transform(_).asInstanceOf[Template]))
236260
}
237261
case tree: ValDef =>

0 commit comments

Comments
 (0)