SI-5918 fixes the ConstantType ugliness

Java enum values are represented with constants wrapping corresponding Symbols.
To find out the underlying type of such a constant one needs to calculate
sym.owner.linkedClassOfClass.tpe (where sym represents the wrapped symbol).

To quote the source code, given (in java): class A { enum E { VAL1 } }
- sym: the symbol of the actual enumeration value (VAL1)
- .owner: the ModuleClassSymbol of the enumeration (object E)
- .linkedClassOfClass: the ClassSymbol of the enumeration (class E)

Back then, as far as I can guess, linkedClassOfClass was flaky and didn't
work well late in the compilation pipeline.

Therefore a fix to SI-1329 introduced a caching facility. Once a ConstantType
representing the type of Constant(sym) was created (I guess, during typer, when
linkedClassOfClass was still working), it cached the underlying type and used
it in subsequent phases.

***

Unfortunately this solution, being fine for enum values, broke another flavor
of constants - type wrapping constants that represent classOf (for example,
Constant(IntTpe) represents the classOf[Int] constant).

Type-wrapping constants are special, because their type (e.g. Class[Int] in the
example from the previous paragraph) changes as the compilation progresses.
Before erasure it's Class[something], and after erasure it's just Class.

Therefore caching types of such constants might lead to incorrect types
flying around after erasure, as described in this scala-internals thread:
http://groups.google.com/group/scala-internals/browse_thread/thread/45185b341aeb6a30.

***

Now when the problem is clear, the question is why didn't it happen before?
That's all because of another peculiarity of the compiler.

Before erasure package references (e.g. in TypeRef prefixes) are represented
as ThisType(sym), where sym stands for a package class symbol. After erasure
such references are represented differently, e.g. java.lang package looks like
TypeRef(TypeRef(TypeRef(NoPrefix, root, Nil), java, Nil), java.lang, Nil).

As described in the aforementioned thread, the incorrect caching strategy
employed in UniqueConstantType mixed with other caching mechanisms in compiler
effectively established a non-clearable cache that goes from Type instances to
types that represent their classOfs, e.g. from String to Class[String].

So if anyone tried to typecheck a classOf after erasure, he/she would get
Class[String] instead of the correct Class, and compiler would crash. Right?

Nope. Before erasure String is TypeRef(ThisType(java.lang), StringSymbol, Nil),
and after erasure it's TypeRef(TypeRef(...), StringSymbol, Nil), as explained
above. Therefore the foul cache would contain two String types: one pre-erasure
going to a pre-erasure Class[String], and another one post-erasure going to
a post-erasure Class.

***

This shaky balance was broken when I tried to implement class tag generation
with shiny Type.erasure method that Martin just exposed in the reflection API.

The erasure method partially invoked the Erasure phase, and for a String
it returned its post-erasure representation (with java.lang prefix represented
as TypeRef, not as ThisType). And after that I used the result of erasure
to build a classOf for a class tag. Since I did it in a macro, it was
typer, a pre-erasure phase.

Now you understand why things broke. That classOf created a Constant
wrapping a post-erasure representation of String, which cached the incorrect
non-erased Class[String] type for a post-erasure type, and things exploded.

You can imagine my panic! The ScalaDays deadline was near, I still had to do
finishing touches to implicit macros (which I actually never had time to do),
and such a fundamental thing exploded.

Actually I figured out the hashing problem, but in the limited time I had
I failed to understand why exactly it's happening, so I introduced the dirty
workaround praised in SI-5918 and moved on.

***

The story doesn't end here.

Some time has passed, and I learned a lot about the compiler. I independently
discovered the ThisType -> TypeRef transform that erasure applies to package
references and patched Type.erasure to undo it. After all, Type.erasure is a
user-facing API, and users don't need to know about post-typer implementation
details. You can read more about this here:
http://groups.google.com/group/scala-internals/browse_thread/thread/6d3277ae21b6d581

From what we've learned above, we can see that this Type.erasure fix made
the UniqueConstantType workaround unnecessary. But I didn't know that.

So imagine my surprise when I tried to remove that workaround and ran the tests
only to see that nothing fails. I went back in time to April when the problem
first manifested, extracted a minimized crasher and tried to use it on trunk.
Again, nothing crashed.

And only with the help of showRaw, I finally understood that types printed as
"String" can be wildly different. The rest was a piece of cake.

***

The irony is that the original reason for ConstantType caching is no longer
valid. linkedClassOfClass now works fine (and files/jvm/outerEnum.scala
agrees with me), so we can remove the cache altogether.

So why all this story about erasure and package references? Well, I don't know.
I enjoyed uncovering this mystery, so I wanted to share it with you :)

3 files changed, 11 insertions, 44 deletions

diff --git a/src/reflect/scala/reflect/internal/Constants.scala b/src/reflect/scala/reflect/internal/Constants.scala
index 61fa553484..4e232e486b 100644
--- a/src/reflect/scala/reflect/internal/Constants.scala
+++ b/src/reflect/scala/reflect/internal/Constants.scala
@@ -73,13 +73,8 @@ trait Constants extends api.Constants {
       case DoubleTag  => DoubleClass.tpe
       case StringTag  => StringClass.tpe
       case NullTag    => NullClass.tpe
-      case ClazzTag   => ClassType(value.asInstanceOf[Type])
-      case EnumTag    =>
-        // given (in java): "class A { enum E { VAL1 } }"
-        //  - symbolValue: the symbol of the actual enumeration value (VAL1)
-        //  - .owner: the ModuleClasSymbol of the enumeration (object E)
-        //  - .linkedClassOfClass: the ClassSymbol of the enumeration (class E)
-        symbolValue.owner.linkedClassOfClass.tpe
+      case ClazzTag   => ClassType(typeValue)
+      case EnumTag    => EnumType(symbolValue)
     }
 
     /** We need the equals method to take account of tags as well as values.
diff --git a/src/reflect/scala/reflect/internal/Definitions.scala b/src/reflect/scala/reflect/internal/Definitions.scala
index 2db8c29a63..b607f8cad9 100644
--- a/src/reflect/scala/reflect/internal/Definitions.scala
+++ b/src/reflect/scala/reflect/internal/Definitions.scala
@@ -713,6 +713,13 @@ trait Definitions extends api.StandardDefinitions {
       if (phase.erasedTypes || forMSIL) ClassClass.tpe
       else appliedType(ClassClass, arg)
 
+    def EnumType(sym: Symbol) =
+      // given (in java): "class A { enum E { VAL1 } }"
+      //  - sym: the symbol of the actual enumeration value (VAL1)
+      //  - .owner: the ModuleClassSymbol of the enumeration (object E)
+      //  - .linkedClassOfClass: the ClassSymbol of the enumeration (class E)
+      sym.owner.linkedClassOfClass.tpe
+
     def vmClassType(arg: Type): Type = ClassType(arg)
     def vmSignature(sym: Symbol, info: Type): String = signature(info)    // !!!
 
diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala
index ee488c9d18..4311f1dd4f 100644
--- a/src/reflect/scala/reflect/internal/Types.scala
+++ b/src/reflect/scala/reflect/internal/Types.scala
@@ -2018,45 +2018,10 @@ trait Types extends api.Types { self: SymbolTable =>
     override def kind = "ConstantType"
   }
 
-  final class UniqueConstantType(value: Constant) extends ConstantType(value) {
-    /** Save the type of `value`. For Java enums, it depends on finding the linked class,
-     *  which might not be found after `flatten`. */
-    private lazy val _tpe: Type = value.tpe
-    override def underlying: Type = _tpe
-  }
+  final class UniqueConstantType(value: Constant) extends ConstantType(value)
 
   object ConstantType extends ConstantTypeExtractor {
-    def apply(value: Constant): ConstantType = {
-      val tpe = new UniqueConstantType(value)
-      if (value.tag == ClazzTag) {
-        // if we carry a classOf, we might be in trouble
-        // http://groups.google.com/group/scala-internals/browse_thread/thread/45185b341aeb6a30
-        // I don't have time for a thorough fix, so I put a hacky workaround here
-        val alreadyThere = uniques findEntry tpe
-        if ((alreadyThere ne null) && (alreadyThere ne tpe) && (alreadyThere.toString != tpe.toString)) {
-          // we need to remove a stale type that has the same hashcode as we do
-          // HashSet doesn't support removal, and this makes our task non-trivial
-          // also we cannot simply recreate it, because that'd skew hashcodes (that change over time, omg!)
-          // the only solution I can see is getting into the underlying array and sneakily manipulating it
-          val ftable = uniques.getClass.getDeclaredFields().find(f => f.getName endsWith "table").get
-          ftable.setAccessible(true)
-          val table = ftable.get(uniques).asInstanceOf[Array[AnyRef]]
-          def overwrite(hc: Int, x: Type) {
-            def index(x: Int): Int = math.abs(x % table.length)
-            var h = index(hc)
-            var entry = table(h)
-            while (entry ne null) {
-              if (x == entry)
-                table(h) = x
-              h = index(h + 1)
-              entry = table(h)
-            }
-          }
-          overwrite(tpe.##, tpe)
-        }
-      }
-      unique(tpe).asInstanceOf[ConstantType]
-    }
+    def apply(value: Constant) = unique(new UniqueConstantType(value))
   }
 
   /* Syncnote: The `volatile` var and `pendingVolatiles` mutable set need not be protected