Skip to content

Commit 64d3b9a

Browse files
committed
Improve comments for the new trait encoding.
1 parent e88c5b7 commit 64d3b9a

File tree

5 files changed

+51
-39
lines changed

5 files changed

+51
-39
lines changed

compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -959,6 +959,7 @@ object BCodeHelpers {
959959
/** An attachment on Apply nodes indicating that it should be compiled with
960960
* `invokespecial` instead of `invokevirtual`. This is used for static
961961
* forwarders.
962+
* See BCodeSkelBuilder.makeStaticForwarder for more details.
962963
*/
963964
val UseInvokeSpecial = new dotc.util.Property.Key[Unit]
964965

compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -497,14 +497,18 @@ trait BCodeSkelBuilder extends BCodeHelpers {
497497
case ValDef(name, tpt, rhs) => () // fields are added in `genPlainClass()`, via `addClassFields()`
498498

499499
case dd: DefDef =>
500+
/* First generate a static forwarder if this is a non-private trait
501+
* trait method. This is required for super calls to this method, which
502+
* go through the static forwarder in order to work around limitations
503+
* of the JVM.
504+
* In theory, this would go in a separate MiniPhase, but it would have to
505+
* sit in a MegaPhase of its own between GenSJSIR and GenBCode, so the cost
506+
* is not worth it. We directly do it in this back-end instead, which also
507+
* kind of makes sense because it is JVM-specific.
508+
*/
500509
val sym = dd.symbol
501-
502-
def needsStaticImplMethod: Boolean =
503-
claszSymbol.isInterface
504-
&& !dd.rhs.isEmpty
505-
&& !sym.isPrivate
506-
&& !sym.isStaticMember
507-
510+
val needsStaticImplMethod =
511+
claszSymbol.isInterface && !dd.rhs.isEmpty && !sym.isPrivate && !sym.isStaticMember
508512
if needsStaticImplMethod then
509513
genStaticForwarderForDefDef(dd)
510514

@@ -550,11 +554,23 @@ trait BCodeSkelBuilder extends BCodeHelpers {
550554
} // end of method initJMethod
551555

552556
private def genStaticForwarderForDefDef(dd: DefDef): Unit =
553-
val forwarderDef = makeStaticForwarder(dd, traitSuperAccessorName(dd.symbol))
557+
val forwarderDef = makeStaticForwarder(dd)
554558
genDefDef(forwarderDef)
555559

556-
private def makeStaticForwarder(dd: DefDef, name: String): DefDef =
560+
/* Generates a synthetic static forwarder for a trait method.
561+
* For a method such as
562+
* def foo(...args: Ts): R
563+
* in trait X, we generate the following method:
564+
* static def foo$($this: X, ...args: Ts): R =
565+
* invokespecial $this.X::foo(...args)
566+
* We force an invokespecial with the attachment UseInvokeSpecial. It is
567+
* necessary to make sure that the call will not follow overrides of foo()
568+
* in subtraits and subclasses, since the whole point of this forward is to
569+
* encode super calls.
570+
*/
571+
private def makeStaticForwarder(dd: DefDef): DefDef =
557572
val origSym = dd.symbol.asTerm
573+
val name = traitSuperAccessorName(origSym)
558574
val info = origSym.info match
559575
case mt: MethodType =>
560576
MethodType(nme.SELF :: mt.paramNames, origSym.owner.typeRef :: mt.paramInfos, mt.resType)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ object AugmentScala2Traits {
2525
* Strangely, Scala 2 super accessors are pickled as private, but are compiled as public expanded.
2626
* In this phase we expand them and make them non-private, so that `ResolveSuper` does something meaningful.
2727
*
28-
* TODO Should we merge this into `ResolveSuper` at this point?
28+
* TODO Should we already drop the Private flag when reading from Scala2 pickles in Scala2Unpickler?
2929
*/
3030
class AugmentScala2Traits extends MiniPhase with IdentityDenotTransformer { thisPhase =>
3131
import ast.tpd._

compiler/src/dotty/tools/dotc/transform/Constructors.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,8 @@ class Constructors extends MiniPhase with IdentityDenotTransformer { thisPhase =
212212
val sym = stat.symbol
213213
assert(isRetained(sym), sym)
214214
if (!stat.rhs.isEmpty && !isWildcardArg(stat.rhs))
215-
/* !!! This should really just be `sym.setter`. However, if we do that, we'll miss
215+
/* !!! Work around #9390
216+
* This should really just be `sym.setter`. However, if we do that, we'll miss
216217
* setters for mixed in `private var`s. Even though the scope clearly contains the
217218
* setter symbol with the correct Name structure (since the `find` finds it),
218219
* `.decl(setterName)` used by `.setter` through `.accessorNamed` will *not* find it.

compiler/src/dotty/tools/dotc/transform/Mixin.scala

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -30,18 +30,13 @@ object Mixin {
3030

3131
/** This phase performs the following transformations:
3232
*
33-
* 1. (done in `traitDefs` and `transformSym`) Map every concrete trait getter
33+
* 1. (done in `traitDefs` and `transformSym`) For every concrete trait getter
3434
*
3535
* <mods> def x(): T = expr
3636
*
37-
* to the pair of definitions:
37+
* make it non-private, and add the definition of its trait setter:
3838
*
39-
* <mods> def x(): T
40-
* protected def initial$x(): T = { stats; expr }
41-
*
42-
* where `stats` comprises all statements between either the start of the trait
43-
* or the previous field definition which are not definitions (i.e. are executed for
44-
* their side effects).
39+
* <mods> def TraitName$_setter_$x(v: T): Unit
4540
*
4641
* 2. (done in `traitDefs`) Make every concrete trait setter
4742
*
@@ -51,61 +46,60 @@ object Mixin {
5146
*
5247
* <mods> def x_=(y: T)
5348
*
54-
* 3. For a non-trait class C:
49+
* 3. (done in `transformSym`) For every module class constructor in traits,
50+
* remove its Private flag (but do not expand its name), since it will have
51+
* to be instantiated in the classes that mix in the trait.
52+
*
53+
* 4. For a non-trait class C:
5554
*
5655
* For every trait M directly implemented by the class (see SymUtils.mixin), in
5756
* reverse linearization order, add the following definitions to C:
5857
*
59-
* 3.1 (done in `traitInits`) For every parameter accessor `<mods> def x(): T` in M,
58+
* 4.1 (done in `traitInits`) For every parameter accessor `<mods> def x(): T` in M,
6059
* in order of textual occurrence, add
6160
*
6261
* <mods> def x() = e
6362
*
6463
* where `e` is the constructor argument in C that corresponds to `x`. Issue
6564
* an error if no such argument exists.
6665
*
67-
* 3.2 (done in `traitInits`) For every concrete trait getter `<mods> def x(): T` in M
66+
* 4.2 (done in `traitInits`) For every concrete trait getter `<mods> def x(): T` in M
6867
* which is not a parameter accessor, in order of textual occurrence, produce the following:
6968
*
70-
* 3.2.1 If `x` is also a member of `C`, and is a lazy val,
69+
* 4.2.1 If `x` is also a member of `C`, and is a lazy val,
7170
*
7271
* <mods> lazy val x: T = super[M].x
7372
*
74-
* 3.2.2 If `x` is also a member of `C`, and M is a Dotty trait,
73+
* 4.2.2 If `x` is also a member of `C`, and is a module,
7574
*
76-
* <mods> def x(): T = super[M].initial$x()
75+
* <mods> lazy module val x: T = new T$(this)
7776
*
78-
* 3.2.3 If `x` is also a member of `C`, and M is a Scala 2.x trait:
77+
* 4.2.3 If `x` is also a member of `C`, and is something else:
7978
*
8079
* <mods> def x(): T = _
8180
*
82-
* 3.2.4 If `x` is not a member of `C`, and M is a Dotty trait:
83-
*
84-
* super[M].initial$x()
85-
*
86-
* 3.2.5 If `x` is not a member of `C`, and M is a Scala2.x trait, nothing gets added.
87-
*
81+
* 4.2.5 If `x` is not a member of `C`, nothing gets added.
8882
*
89-
* 3.3 (done in `superCallOpt`) The call:
83+
* 4.3 (done in `superCallOpt`) The call:
9084
*
91-
* super[M].<init>
85+
* super[M].$init$()
9286
*
93-
* 3.4 (done in `setters`) For every concrete setter `<mods> def x_=(y: T)` in M:
87+
* 4.4 (done in `setters`) For every concrete setter `<mods> def x_=(y: T)` in M:
9488
*
9589
* <mods> def x_=(y: T) = ()
9690
*
97-
* 3.5 (done in `mixinForwarders`) For every method
91+
* 4.5 (done in `mixinForwarders`) For every method
9892
* `<mods> def f[Ts](ps1)...(psN): U` imn M` that needs to be disambiguated:
9993
*
10094
* <mods> def f[Ts](ps1)...(psN): U = super[M].f[Ts](ps1)...(psN)
10195
*
10296
* A method in M needs to be disambiguated if it is concrete, not overridden in C,
10397
* and if it overrides another concrete method.
10498
*
105-
* 4. (done in `transformTemplate` and `transformSym`) Drop all parameters from trait
106-
* constructors.
99+
* 5. (done in `transformTemplate` and `transformSym`) Drop all parameters from trait
100+
* constructors, and rename them to `nme.TRAIT_CONSTRUCTOR`.
107101
*
108-
* 5. (done in `transformSym`) Drop ParamAccessor flag from all parameter accessors in traits.
102+
* 6. (done in `transformSym`) Drop ParamAccessor flag from all parameter accessors in traits.
109103
*
110104
* Conceptually, this is the second half of the previous mixin phase. It needs to run
111105
* after erasure because it copies references to possibly private inner classes and objects

0 commit comments

Comments
 (0)