Cleanups in Names.scala

Enter only fully constructed Name objects into the hash table, this
should make lookupTypeName thread-safe.

Clean up lookupTypeName - the hash code for a type name is the same
as for its correspondent term name.

Going from a type name toTermName should never create a new TermName
instance. Assert that.

1 files changed, 38 insertions, 53 deletions

diff --git a/src/reflect/scala/reflect/internal/Names.scala b/src/reflect/scala/reflect/internal/Names.scala
index fce1fc2a0c..05c060e81f 100644
--- a/src/reflect/scala/reflect/internal/Names.scala
+++ b/src/reflect/scala/reflect/internal/Names.scala
@@ -109,8 +109,13 @@ trait Names extends api.Names {
         // to enterChars cannot follow the construction of the TermName.
         val ncStart = nc
         enterChars(cs, offset, len)
-        if (cachedString ne null) new TermName_S(ncStart, len, h, cachedString)
-        else new TermName_R(ncStart, len, h)
+        val next = termHashtable(h)
+        val termName =
+          if (cachedString ne null) new TermName_S(ncStart, len, next, cachedString)
+          else new TermName_R(ncStart, len, next)
+        // Add the new termName to the hashtable only after it's been fully constructed
+        termHashtable(h) = termName
+        termName
       }
     }
     if (synchronizeNames) nameLock.synchronized(body) else body
@@ -148,40 +153,20 @@ trait Names extends api.Names {
     newTermName(bs, offset, len).toTypeName
 
   /**
-   *  Used only by the GenBCode backend, to represent bytecode-level types in a way that makes equals() and hashCode() efficient.
-   *  For bytecode-level types of OBJECT sort, its internal name (not its descriptor) is stored.
-   *  For those of ARRAY sort,  its descriptor is stored ie has a leading '['
-   *  For those of METHOD sort, its descriptor is stored ie has a leading '('
+   * Used by the GenBCode backend to lookup type names that are known to already exist. This method
+   * might be invoked in a multi-threaded setting. Invoking newTypeName instead might be unsafe.
    *
-   *  can-multi-thread
-   *  TODO SI-6240 !!! JZ Really? the constructors TermName and TypeName publish unconstructed `this` references
-   *               into the hash tables; we could observe them here before the subclass constructor completes.
+   * can-multi-thread: names are added to the hash tables only after they are fully constructed.
    */
-  final def lookupTypeName(cs: Array[Char]): TypeName = { lookupTypeNameIfExisting(cs, true) }
+  final def lookupTypeName(cs: Array[Char]): TypeName = {
+    val hash = hashValue(cs, 0, cs.length) & HASH_MASK
+    var typeName = typeHashtable(hash)
 
-  final def lookupTypeNameIfExisting(cs: Array[Char], failOnNotFound: Boolean): TypeName = {
-
-    val hterm = hashValue(cs, 0, cs.length) & HASH_MASK
-    var nterm = termHashtable(hterm)
-    while ((nterm ne null) && (nterm.length != cs.length || !equals(nterm.start, cs, 0, cs.length))) {
-      nterm = nterm.next
-    }
-    if (nterm eq null) {
-      if (failOnNotFound) { assert(false, "TermName not yet created: " + new String(cs)) }
-      return null
-    }
-
-    val htype = hashValue(chrs, nterm.start, nterm.length) & HASH_MASK
-    var ntype = typeHashtable(htype)
-    while ((ntype ne null) && ntype.start != nterm.start) {
-      ntype = ntype.next
-    }
-    if (ntype eq null) {
-      if (failOnNotFound) { assert(false, "TypeName not yet created: " + new String(cs)) }
-      return null
+    while ((typeName ne null) && (typeName.length != cs.length || !equals(typeName.start, cs, 0, cs.length))) {
+      typeName = typeName.next
     }
-
-    ntype
+    assert(typeName != null, s"TypeName ${new String(cs)} not yet created.")
+    typeName
   }
 
 // Classes ----------------------------------------------------------------------
@@ -518,43 +503,47 @@ trait Names extends api.Names {
   /** TermName_S and TypeName_S have fields containing the string version of the name.
    *  TermName_R and TypeName_R recreate it each time toString is called.
    */
-  private final class TermName_S(index0: Int, len0: Int, hash: Int, override val toString: String) extends TermName(index0, len0, hash) {
-    protected def createCompanionName(h: Int): TypeName = new TypeName_S(index, len, h, toString)
+  private final class TermName_S(index0: Int, len0: Int, next0: TermName, override val toString: String) extends TermName(index0, len0, next0) {
+    protected def createCompanionName(next: TypeName): TypeName = new TypeName_S(index, len, next, toString)
     override def newName(str: String): TermName = newTermNameCached(str)
   }
-  private final class TypeName_S(index0: Int, len0: Int, hash: Int, override val toString: String) extends TypeName(index0, len0, hash) {
-    protected def createCompanionName(h: Int): TermName = new TermName_S(index, len, h, toString)
+  private final class TypeName_S(index0: Int, len0: Int, next0: TypeName, override val toString: String) extends TypeName(index0, len0, next0) {
     override def newName(str: String): TypeName = newTypeNameCached(str)
   }
 
-  private final class TermName_R(index0: Int, len0: Int, hash: Int) extends TermName(index0, len0, hash) {
-    protected def createCompanionName(h: Int): TypeName = new TypeName_R(index, len, h)
+  private final class TermName_R(index0: Int, len0: Int, next0: TermName) extends TermName(index0, len0, next0) {
+    protected def createCompanionName(next: TypeName): TypeName = new TypeName_R(index, len, next)
     override def toString = new String(chrs, index, len)
   }
 
-  private final class TypeName_R(index0: Int, len0: Int, hash: Int) extends TypeName(index0, len0, hash) {
-    protected def createCompanionName(h: Int): TermName = new TermName_R(index, len, h)
+  private final class TypeName_R(index0: Int, len0: Int, next0: TypeName) extends TypeName(index0, len0, next0) {
     override def toString = new String(chrs, index, len)
   }
 
   // SYNCNOTE: caller to constructor must synchronize if `synchronizeNames` is enabled
-  sealed abstract class TermName(index0: Int, len0: Int, hash: Int) extends Name(index0, len0) with TermNameApi {
+  sealed abstract class TermName(index0: Int, len0: Int, val next: TermName) extends Name(index0, len0) with TermNameApi {
     type ThisNameType = TermName
     protected[this] def thisName: TermName = this
-    val next: TermName = termHashtable(hash)
-    termHashtable(hash) = this
+
     def isTermName: Boolean = true
     def isTypeName: Boolean = false
     def toTermName: TermName = this
     def toTypeName: TypeName = {
       def body = {
+        // Re-computing the hash saves a field for storing it in the TermName
         val h = hashValue(chrs, index, len) & HASH_MASK
         var n = typeHashtable(h)
         while ((n ne null) && n.start != index)
           n = n.next
 
         if (n ne null) n
-        else createCompanionName(h)
+        else {
+          val next = typeHashtable(h)
+          val typeName = createCompanionName(next)
+          // Add the new typeName to the hashtable only after it's been fully constructed
+          typeHashtable(h) = typeName
+          typeName
+        }
       }
       if (synchronizeNames) nameLock.synchronized(body) else body
     }
@@ -565,7 +554,7 @@ trait Names extends api.Names {
 
     def nameKind = "term"
     /** SYNCNOTE: caller must synchronize if `synchronizeNames` is enabled */
-    protected def createCompanionName(h: Int): TypeName
+    protected def createCompanionName(next: TypeName): TypeName
   }
 
   implicit val TermNameTag = ClassTag[TermName](classOf[TermName])
@@ -575,24 +564,22 @@ trait Names extends api.Names {
     def unapply(name: TermName): Option[String] = Some(name.toString)
   }
 
-  sealed abstract class TypeName(index0: Int, len0: Int, hash: Int) extends Name(index0, len0) with TypeNameApi {
+  sealed abstract class TypeName(index0: Int, len0: Int, val next: TypeName) extends Name(index0, len0) with TypeNameApi {
     type ThisNameType = TypeName
     protected[this] def thisName: TypeName = this
 
-    val next: TypeName = typeHashtable(hash)
-    typeHashtable(hash) = this
-
     def isTermName: Boolean = false
     def isTypeName: Boolean = true
     def toTermName: TermName = {
       def body = {
+        // Re-computing the hash saves a field for storing it in the TypeName
         val h = hashValue(chrs, index, len) & HASH_MASK
         var n = termHashtable(h)
         while ((n ne null) && n.start != index)
           n = n.next
 
-        if (n ne null) n
-        else createCompanionName(h)
+        assert (n ne null, s"TypeName $this is missing its correspondent")
+        n
       }
       if (synchronizeNames) nameLock.synchronized(body) else body
     }
@@ -604,8 +591,6 @@ trait Names extends api.Names {
 
     def nameKind = "type"
     override def decode = if (nameDebug) super.decode + "!" else super.decode
-    /** SYNCNOTE: caller must synchronize if `synchronizeNames` is enabled */
-    protected def createCompanionName(h: Int): TermName
   }
 
   implicit val TypeNameTag = ClassTag[TypeName](classOf[TypeName])