Skip to content

Commit 2e7c734

Browse files
committed
SI-7974 Clean up and test 'Symbol-handling code in CleanUp
Looks like the transformation did never happen because the pattern failed to match. Why did the pattern fail to match? Because the Symbol.apply we see in the tree claims to be a method while Symbol_apply defined in Definitions wants to be a value. This issue was caused because nonPrivateMember starts spitting out overloaded symbols after erasure. This issue has been fixed in the earlier commit, so what happens in this commit is adding tests and fixing documentation.
1 parent 5e1e472 commit 2e7c734

File tree

4 files changed

+159
-14
lines changed

4 files changed

+159
-14
lines changed

src/compiler/scala/tools/nsc/transform/CleanUp.scala

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -481,18 +481,33 @@ abstract class CleanUp extends Statics with Transform with ast.TreeDSL {
481481
* For instance, say we have a Scala class:
482482
*
483483
* class Cls {
484-
* // ...
485-
* def someSymbol = `symbolic
486-
* // ...
484+
* def someSymbol1 = 'Symbolic1
485+
* def someSymbol2 = 'Symbolic2
486+
* def sameSymbol1 = 'Symbolic1
487+
* val someSymbol3 = 'Symbolic3
487488
* }
488489
*
489490
* After transformation, this class looks like this:
490491
*
491492
* class Cls {
492-
* private "static" val <some_name>$symbolic = Symbol("symbolic")
493-
* // ...
494-
* def someSymbol = <some_name>$symbolic
495-
* // ...
493+
* private <static> var symbol$1: scala.Symbol
494+
* private <static> var symbol$2: scala.Symbol
495+
* private <static> var symbol$3: scala.Symbol
496+
* private val someSymbol3: scala.Symbol
497+
*
498+
* private <static> def <clinit> = {
499+
* symbol$1 = Symbol.apply("Symbolic1")
500+
* symbol$2 = Symbol.apply("Symbolic2")
501+
* }
502+
*
503+
* private def <init> = {
504+
* someSymbol3 = symbol$3
505+
* }
506+
*
507+
* def someSymbol1 = symbol$1
508+
* def someSymbol2 = symbol$2
509+
* def sameSymbol1 = symbol$1
510+
* val someSymbol3 = someSymbol3
496511
* }
497512
*
498513
* The reasoning behind this transformation is the following. Symbols get interned - they are stored
@@ -502,17 +517,17 @@ abstract class CleanUp extends Statics with Transform with ast.TreeDSL {
502517
* is accessed only once during class loading, and after that, the unique symbol is in the static
503518
* member. Hence, it is cheap to both reach the unique symbol and do equality checks on it.
504519
*
505-
* And, finally, be advised - scala symbol literal and the Symbol class of the compiler
520+
* And, finally, be advised - Scala's Symbol literal (scala.Symbol) and the Symbol class of the compiler
506521
* have little in common.
507522
*/
508523
case Apply(fn, (arg @ Literal(Constant(symname: String))) :: Nil) if fn.symbol == Symbol_apply =>
509524
def transformApply = {
510-
// add the symbol name to a map if it's not there already
511-
val rhs = gen.mkMethodCall(Symbol_apply, arg :: Nil)
512-
val staticFieldSym = getSymbolStaticField(tree.pos, symname, rhs, tree)
513-
// create a reference to a static field
514-
val ntree = typedWithPos(tree.pos)(REF(staticFieldSym))
515-
super.transform(ntree)
525+
// add the symbol name to a map if it's not there already
526+
val rhs = gen.mkMethodCall(Symbol_apply, arg :: Nil)
527+
val staticFieldSym = getSymbolStaticField(tree.pos, symname, rhs, tree)
528+
// create a reference to a static field
529+
val ntree = typedWithPos(tree.pos)(REF(staticFieldSym))
530+
super.transform(ntree)
516531
}
517532
transformApply
518533

test/files/run/t7974.check

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
public class Symbols {
2+
3+
// compiled from: Symbols.scala
4+
5+
6+
7+
// access flags 0x12
8+
private final Lscala/Symbol; someSymbol3
9+
10+
// access flags 0xA
11+
private static Lscala/Symbol; symbol$1
12+
13+
// access flags 0xA
14+
private static Lscala/Symbol; symbol$2
15+
16+
// access flags 0xA
17+
private static Lscala/Symbol; symbol$3
18+
19+
// access flags 0x9
20+
public static <clinit>()V
21+
L0
22+
LINENUMBER 2 L0
23+
GETSTATIC scala/Symbol$.MODULE$ : Lscala/Symbol$;
24+
LDC "Symbolic1"
25+
INVOKEVIRTUAL scala/Symbol$.apply (Ljava/lang/String;)Lscala/Symbol;
26+
PUTSTATIC Symbols.symbol$1 : Lscala/Symbol;
27+
L1
28+
LINENUMBER 3 L1
29+
GETSTATIC scala/Symbol$.MODULE$ : Lscala/Symbol$;
30+
LDC "Symbolic2"
31+
INVOKEVIRTUAL scala/Symbol$.apply (Ljava/lang/String;)Lscala/Symbol;
32+
PUTSTATIC Symbols.symbol$2 : Lscala/Symbol;
33+
L2
34+
LINENUMBER 5 L2
35+
GETSTATIC scala/Symbol$.MODULE$ : Lscala/Symbol$;
36+
LDC "Symbolic3"
37+
INVOKEVIRTUAL scala/Symbol$.apply (Ljava/lang/String;)Lscala/Symbol;
38+
PUTSTATIC Symbols.symbol$3 : Lscala/Symbol;
39+
RETURN
40+
MAXSTACK = 2
41+
MAXLOCALS = 0
42+
43+
// access flags 0x1
44+
public someSymbol1()Lscala/Symbol;
45+
L0
46+
LINENUMBER 2 L0
47+
GETSTATIC Symbols.symbol$1 : Lscala/Symbol;
48+
ARETURN
49+
L1
50+
LOCALVARIABLE this LSymbols; L0 L1 0
51+
MAXSTACK = 1
52+
MAXLOCALS = 1
53+
54+
// access flags 0x1
55+
public someSymbol2()Lscala/Symbol;
56+
L0
57+
LINENUMBER 3 L0
58+
GETSTATIC Symbols.symbol$2 : Lscala/Symbol;
59+
ARETURN
60+
L1
61+
LOCALVARIABLE this LSymbols; L0 L1 0
62+
MAXSTACK = 1
63+
MAXLOCALS = 1
64+
65+
// access flags 0x1
66+
public sameSymbol1()Lscala/Symbol;
67+
L0
68+
LINENUMBER 4 L0
69+
GETSTATIC Symbols.symbol$1 : Lscala/Symbol;
70+
ARETURN
71+
L1
72+
LOCALVARIABLE this LSymbols; L0 L1 0
73+
MAXSTACK = 1
74+
MAXLOCALS = 1
75+
76+
// access flags 0x1
77+
public someSymbol3()Lscala/Symbol;
78+
L0
79+
LINENUMBER 5 L0
80+
ALOAD 0
81+
GETFIELD Symbols.someSymbol3 : Lscala/Symbol;
82+
ARETURN
83+
L1
84+
LOCALVARIABLE this LSymbols; L0 L1 0
85+
MAXSTACK = 1
86+
MAXLOCALS = 1
87+
88+
// access flags 0x1
89+
public <init>()V
90+
L0
91+
LINENUMBER 6 L0
92+
ALOAD 0
93+
INVOKESPECIAL java/lang/Object.<init> ()V
94+
L1
95+
LINENUMBER 5 L1
96+
ALOAD 0
97+
GETSTATIC Symbols.symbol$3 : Lscala/Symbol;
98+
PUTFIELD Symbols.someSymbol3 : Lscala/Symbol;
99+
RETURN
100+
L2
101+
LOCALVARIABLE this LSymbols; L0 L2 0
102+
MAXSTACK = 2
103+
MAXLOCALS = 1
104+
}

test/files/run/t7974/Symbols.scala

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
class Symbols {
2+
def someSymbol1 = 'Symbolic1
3+
def someSymbol2 = 'Symbolic2
4+
def sameSymbol1 = 'Symbolic1
5+
val someSymbol3 = 'Symbolic3
6+
}

test/files/run/t7974/Test.scala

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import java.io.PrintWriter;
2+
3+
import scala.tools.partest.BytecodeTest
4+
import scala.tools.asm.util._
5+
import scala.tools.nsc.util.stringFromWriter
6+
7+
object Test extends BytecodeTest {
8+
def show {
9+
val classNode = loadClassNode("Symbols", skipDebugInfo = false)
10+
val textifier = new Textifier
11+
classNode.accept(new TraceClassVisitor(null, textifier, null))
12+
13+
val classString = stringFromWriter(w => textifier.print(w))
14+
val result =
15+
classString.split('\n')
16+
.dropWhile(elem => elem != "public class Symbols {")
17+
.filterNot(elem => elem.startsWith(" @Lscala/reflect/ScalaSignature") || elem.startsWith(" ATTRIBUTE ScalaSig"))
18+
result foreach println
19+
}
20+
}

0 commit comments

Comments
 (0)