Skip to content

Commit 17d9706

Browse files
lrytzlrytz
committed
Fix InlineInfo attribute for nested module accessors
1 parent 2ed0d2a commit 17d9706

File tree

2 files changed

+74
-34
lines changed

2 files changed

+74
-34
lines changed

‎src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -263,11 +263,13 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
263263
val name = methodSym.javaSimpleName.toString // same as in genDefDef
264264
val signature = name + methodSymToDescriptor(methodSym)
265265

266-
// Some detours are required here because of changing flags (lateDEFERRED):
267-
// 1. Why the phase travel? Concrete trait methods obtain the lateDEFERRED flag in Mixin.
268-
// This makes isEffectivelyFinalOrNotOverridden false, which would prevent non-final
269-
// but non-overridden methods of sealed traits from being inlined.
270-
val effectivelyFinal = exitingPickler(methodSym.isEffectivelyFinalOrNotOverridden) && !(methodSym.owner.isTrait && methodSym.isModule)
266+
// In `trait T { object O }`, `oSym.isEffectivelyFinalOrNotOverridden` is true, but the
267+
// method is abstract in bytecode, `defDef.rhs.isEmpty`. Abstract methods are excluded
268+
// so they are not marked final in the InlineInfo attribute.
269+
//
270+
// However, due to https://github.com/scala/scala-dev/issues/126, this currently does not
271+
// work, the abstract accessor for O will be marked effectivelyFinal.
272+
val effectivelyFinal = methodSym.isEffectivelyFinalOrNotOverridden && !methodSym.isDeferred
271273

272274
val info = MethodInlineInfo(
273275
effectivelyFinal = effectivelyFinal,

‎test/junit/scala/tools/nsc/backend/jvm/opt/ScalaInlineInfoTest.scala‎

Lines changed: 67 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,24 @@ class ScalaInlineInfoTest extends ClearAfterClass {
2929

3030
def inlineInfo(c: ClassNode): InlineInfo = c.attrs.asScala.collect({ case a: InlineInfoAttribute => a.inlineInfo }).head
3131

32+
def mapDiff[A, B](a: Map[A, B], b: Map[A, B]) = {
33+
val r = new StringBuilder
34+
for ((a, av) <- a) {
35+
if (!b.contains(a)) r.append(s"missing in b: $a\n")
36+
else if (av != b(a)) r.append(s"different for $a: $av != ${b(a)}\n")
37+
}
38+
for (b <- b.keys.toList diff a.keys.toList) {
39+
r.append(s"missing in a: $b\n")
40+
}
41+
r.toString
42+
}
43+
3244
@Test
3345
def traitMembersInlineInfo(): Unit = {
3446
val code =
3547
"""trait T {
3648
| def f1 = 1 // concrete method
37-
| private def f2 = 1 // implOnly method (does not end up in the interface)
49+
| private def f2 = 1 // default method only (not in subclass)
3850
| def f3 = {
3951
| def nest = 0 // nested method (does not end up in the interface)
4052
| nest
@@ -44,13 +56,13 @@ class ScalaInlineInfoTest extends ClearAfterClass {
4456
| def f4 = super.toString // super accessor
4557
|
4658
| object O // module accessor (method is generated)
47-
| def f5 = {
59+
| final def f5 = {
4860
| object L { val x = 0 } // nested module (just flattened out)
4961
| L.x
5062
| }
5163
|
5264
| @noinline
53-
| def f6: Int // abstract method (not in impl class)
65+
| def f6: Int // abstract method
5466
|
5567
| // fields
5668
|
@@ -61,39 +73,65 @@ class ScalaInlineInfoTest extends ClearAfterClass {
6173
|
6274
| final val x5 = 0
6375
|}
76+
|class C extends T {
77+
| def f6 = 0
78+
| var x3 = 0
79+
|}
6480
""".stripMargin
6581

66-
val cs @ List(t, tl, to) = compileClasses(compiler)(code)
67-
val info = inlineInfo(t)
68-
val expect = InlineInfo (
82+
val cs @ List(c, t, tl, to) = compileClasses(compiler)(code)
83+
val infoT = inlineInfo(t)
84+
val expectT = InlineInfo (
6985
false, // final class
7086
None, // not a sam
7187
Map(
72-
// TODO SD-86: the module accessor used to be `effectivelyFinal` before nuke-impl-classes
73-
("O()LT$O$;", MethodInlineInfo(false,false,false)),
74-
("T$$super$toString()Ljava/lang/String;", MethodInlineInfo(false,false,false)),
75-
("T$_setter_$x1_$eq(I)V", MethodInlineInfo(false,false,false)),
76-
("f1()I", MethodInlineInfo(false,false,false)),
77-
("f3()I", MethodInlineInfo(false,false,false)),
78-
("f4()Ljava/lang/String;", MethodInlineInfo(false,true, false)),
79-
("f5()I", MethodInlineInfo(false,false,false)),
80-
("f6()I", MethodInlineInfo(false,false,true )),
81-
("x1()I", MethodInlineInfo(false,false,false)),
82-
("x3()I", MethodInlineInfo(false,false,false)),
83-
("x3_$eq(I)V", MethodInlineInfo(false,false,false)),
84-
("x4()I", MethodInlineInfo(false,false,false)),
85-
("x5()I", MethodInlineInfo(true, false,false)),
86-
("y2()I", MethodInlineInfo(false,false,false)),
87-
("y2_$eq(I)V", MethodInlineInfo(false,false,false)),
88-
("f2()I", MethodInlineInfo(true, false,false)),
89-
("L$lzycompute$1(Lscala/runtime/VolatileObjectRef;)LT$L$2$;",MethodInlineInfo(true, false,false)),
90-
// TODO SD-86: should probably be effectivelyFinal
91-
("L$1(Lscala/runtime/VolatileObjectRef;)LT$L$2$;", MethodInlineInfo(false,false,false)),
92-
("nest$1()I", MethodInlineInfo(true, false,false)),
93-
("$init$()V", MethodInlineInfo(false,false,false))),
88+
("O()LT$O$;", MethodInlineInfo(true ,false,false)), // the accessor is abstract in bytecode, but still effectivelyFinal because there's no (late)DEFERRED flag, https://github.com/scala/scala-dev/issues/126
89+
("T$$super$toString()Ljava/lang/String;", MethodInlineInfo(true ,false,false)),
90+
("T$_setter_$x1_$eq(I)V", MethodInlineInfo(false,false,false)),
91+
("f1()I", MethodInlineInfo(false,false,false)),
92+
("f2()I", MethodInlineInfo(true, false,false)),
93+
("f3()I", MethodInlineInfo(false,false,false)),
94+
("f4()Ljava/lang/String;", MethodInlineInfo(false,true, false)),
95+
("f5()I", MethodInlineInfo(true ,false,false)),
96+
("f6()I", MethodInlineInfo(false,false,true )),
97+
("x1()I", MethodInlineInfo(false,false,false)),
98+
("y2()I", MethodInlineInfo(false,false,false)),
99+
("y2_$eq(I)V", MethodInlineInfo(false,false,false)),
100+
("x3()I", MethodInlineInfo(false,false,false)),
101+
("x3_$eq(I)V", MethodInlineInfo(false,false,false)),
102+
("x4()I", MethodInlineInfo(false,false,false)),
103+
("x5()I", MethodInlineInfo(true, false,false)),
104+
("L$lzycompute$1(Lscala/runtime/VolatileObjectRef;)LT$L$2$;", MethodInlineInfo(true, false,false)),
105+
("L$1(Lscala/runtime/VolatileObjectRef;)LT$L$2$;", MethodInlineInfo(true ,false,false)),
106+
("nest$1()I", MethodInlineInfo(true, false,false)),
107+
("$init$()V", MethodInlineInfo(false,false,false))),
94108
None // warning
95109
)
96-
assert(info == expect, info)
110+
111+
assert(infoT == expectT, mapDiff(expectT.methodInfos, infoT.methodInfos) + infoT)
112+
113+
val infoC = inlineInfo(c)
114+
val expectC = InlineInfo(false, None, Map(
115+
"O()LT$O$;" -> MethodInlineInfo(true ,false,false),
116+
"f1()I" -> MethodInlineInfo(false,false,false),
117+
"f3()I" -> MethodInlineInfo(false,false,false),
118+
"f4()Ljava/lang/String;" -> MethodInlineInfo(false,true ,false),
119+
"f5()I" -> MethodInlineInfo(true ,false,false),
120+
"f6()I" -> MethodInlineInfo(false,false,false),
121+
"x1()I" -> MethodInlineInfo(false,false,false),
122+
"T$_setter_$x1_$eq(I)V" -> MethodInlineInfo(false,false,false),
123+
"y2()I" -> MethodInlineInfo(false,false,false),
124+
"y2_$eq(I)V" -> MethodInlineInfo(false,false,false),
125+
"x3()I" -> MethodInlineInfo(false,false,false),
126+
"x3_$eq(I)V" -> MethodInlineInfo(false,false,false),
127+
"x4$lzycompute()I" -> MethodInlineInfo(true ,false,false),
128+
"x4()I" -> MethodInlineInfo(false,false,false),
129+
"x5()I" -> MethodInlineInfo(true ,false,false),
130+
"T$$super$toString()Ljava/lang/String;" -> MethodInlineInfo(true ,false,false),
131+
"<init>()V" -> MethodInlineInfo(false,false,false)),
132+
None)
133+
134+
assert(infoC == expectC, mapDiff(expectC.methodInfos, infoC.methodInfos) + infoC)
97135
}
98136

99137
@Test

0 commit comments

Comments
 (0)