Avoid cycles and some needless class reading in Java classfile parsing

Manually tested with:

```
$ scalac -cp $(coursier fetch -q -p com.datastax.cassandra:dse-driver:1.0.0) test.scala
test.scala:2: error: illegal cyclic reference involving class Cluster
	new com.datastax.driver.dse.DseCluster.Builder()
                                               ^
one error found

$ /code/scala/build/quick/bin/scalac -cp $(coursier fetch -q -p com.datastax.cassandra:dse-driver:1.0.0) test.scala

$ cat test.scala
class Test {
	new com.datastax.driver.dse.DseCluster.Builder()
}
```

```
class Test {
  new com.datastax.driver.dse.DseCluster.Builder()
}
```

I was also able to compile `pos/cycle{-jsoup,}` without the experimental
`-Ybreak-cycles`. I've removed the entire `-Ybreak-cycles` option and
supporting code in favour of the approach in this patch.

What's changed? I've used lazy types for methods infos, which is analagous
to what we do in `Unpickler` for scala originated types.

Fixes scala/bug#3809

Author: retronym

src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala

@@ -519,7 +519,8 @@ abstract class ClassfileParser {
       skipMembers() // methods
 
       // attributes now depend on having infos set already
-      parseAttributes(clazz, classInfo)
+      val paramNames = ListBuffer[Name](null)
+      parseAttributes(clazz, isVarargs = false)
 
       def queueLoad(): Unit = {
         in.bp = fieldsStartBp
@@ -576,7 +577,7 @@ abstract class ClassfileParser {
         else info
       }
       propagatePackageBoundary(jflags, sym)
-      parseAttributes(sym, info)
+      parseAttributes(sym, isVarargs = false)
       addJavaFlagsAnnotations(sym, jflags)
       getScope(jflags) enter sym
 
@@ -611,47 +612,13 @@ abstract class ClassfileParser {
       } else {
         val name = readName()
         val sym = ownerForFlags(jflags).newMethod(name.toTermName, NoPosition, sflags)
-        var info = pool.getType(sym, u2)
-        var removedOuterParameter = false
-        if (name == nme.CONSTRUCTOR)
-          info match {
-            case MethodType(params, restpe) =>
-              // if this is a non-static inner class, remove the explicit outer parameter
-              val paramsNoOuter = innerClasses getEntry currentClass match {
-                case Some(entry) if !entry.jflags.isStatic =>
-                  /* About `clazz.owner.hasPackageFlag` below: scala/bug#5957
-                   * For every nested java class A$B, there are two symbols in the scala compiler.
-                   *  1. created by SymbolLoader, because of the existence of the A$B.class file, owner: package
-                   *  2. created by ClassfileParser of A when reading the inner classes, owner: A
-                   * If symbol 1 gets completed (e.g. because the compiled source mentions `A$B`, not `A#B`), the
-                   * ClassfileParser for 1 executes, and clazz.owner is the package.
-                   */
-                  assert(params.head.tpe.typeSymbol == clazz.owner || clazz.owner.hasPackageFlag, params.head.tpe.typeSymbol + ": " + clazz.owner)
-                  removedOuterParameter = true
-                  params.tail
-                case _ =>
-                  params
-              }
-              val newParams = paramsNoOuter match {
-                case (init :+ tail) if jflags.isSynthetic =>
-                  // scala/bug#7455 strip trailing dummy argument ("access constructor tag") from synthetic constructors which
-                  // are added when an inner class needs to access a private constructor.
-                  init
-                case _ =>
-                  paramsNoOuter
-              }
-
-              info = MethodType(newParams, clazz.tpe)
-          }
         // Note: the info may be overwritten later with a generic signature
         // parsed from SignatureATTR
-        sym setInfo info
+        val lazyInfo = new JavaSignatureTypeCompleter(name, jflags, index = u2)
+        sym.info = lazyInfo
         propagatePackageBoundary(jflags, sym)
-        parseAttributes(sym, info, removedOuterParameter)
+        parseAttributes(sym, jflags.isVarargs)
         addJavaFlagsAnnotations(sym, jflags)
-        if (jflags.isVarargs)
-          sym modifyInfo arrayToRepeated
-
         getScope(jflags) enter sym
       }
     }
@@ -835,8 +802,7 @@ abstract class ClassfileParser {
   /**
    * Only invoked for java classfiles.
    */
-  def parseAttributes(sym: Symbol, symtype: Type, removedOuterParameter: Boolean = false): Unit = {
-    var paramNames: ListBuffer[Name] = null // null means we didn't find any
+  def parseAttributes(sym: symbolTable.Symbol, isVarargs: Boolean = false): Unit = {
     def convertTo(c: Constant, pt: Type): Constant = {
       if (pt.typeSymbol == BooleanClass && c.tag == IntTag)
         Constant(c.value != 0)
@@ -850,9 +816,12 @@ abstract class ClassfileParser {
       attrName match {
         case tpnme.SignatureATTR =>
           val sig = pool.getExternalName(u2)
-          val newType = sigToType(sym, sig.value)
-          sym.setInfo(newType)
-
+          if (sym.isClass) {
+            sym.setInfo(sigToType(sym, sig.value))
+          } else {
+            val lazyType = new JavaGenericSignatureTypeCompleter(sig.value, isVarargs)
+            sym.setInfo(lazyType)
+          }
         case tpnme.SyntheticATTR =>
           sym.setFlag(SYNTHETIC | ARTIFACT)
           in.skip(attrLen)
@@ -868,20 +837,16 @@ abstract class ClassfileParser {
 
         case tpnme.ConstantValueATTR =>
           val c = pool.getConstant(u2)
-          val c1 = convertTo(c, symtype)
+          val c1 = convertTo(c, sym.initialize.info.resultType)
           if (c1 ne null) sym.setInfo(ConstantType(c1))
-          else devWarning(s"failure to convert $c to $symtype")
+          else devWarning(s"failure to convert $c to ${sym.initialize.info.resultType}")
 
         case tpnme.MethodParametersATTR =>
           def readParamNames(): Unit = {
             import scala.tools.asm.Opcodes.ACC_SYNTHETIC
+            val paramNames = ListBuffer[Name]()
             val paramCount = u1
             var i = 0
-            if (removedOuterParameter && i < paramCount) {
-              in.skip(4)
-              i += 1
-            }
-            paramNames = new ListBuffer()
             while (i < paramCount) {
               val rawname = pool.getName(u2)
               val access = u2
@@ -893,6 +858,7 @@ abstract class ClassfileParser {
               paramNames += name
               i += 1
             }
+            sym.updateAttachment(new ParamNames(paramNames.toList))
           }
           readParamNames()
 
@@ -963,26 +929,8 @@ abstract class ClassfileParser {
         sym.addThrowsAnnotation(cls)
       }
     }
-    def addParamNames(): Unit =
-      if ((paramNames ne null) && sym.hasRawInfo && sym.isMethod) {
-        val params = sym.rawInfo.params
-        (paramNames zip params).foreach {
-          case (nme.NO_NAME, _) => // param was ACC_SYNTHETIC; ignore
-          case (name, param) =>
-            param.resetFlag(SYNTHETIC)
-            param.name = name
-        }
-        if (isDeveloper && !sameLength(paramNames.toList, params)) {
-          // there's not anything we can do, but it's slightly worrisome
-          devWarning(
-            sm"""MethodParameters length mismatch while parsing $sym:
-                |  rawInfo.params: ${sym.rawInfo.params}
-                |  MethodParameters: ${paramNames.toList}""")
-        }
-      }
     // begin parseAttributes
     for (i <- 0 until u2) parseAttribute()
-    addParamNames()
   }
 
 
@@ -1313,6 +1261,76 @@ abstract class ClassfileParser {
       sym setInfo createFromClonedSymbols(alias.initialize.typeParams, alias.tpe)(typeFun)
     }
   }
+  final class JavaSignatureTypeCompleter(name: Name, jflags: JavaAccFlags, index: Int) extends LazyType {
+    override def complete(sym: symbolTable.Symbol): Unit = {
+      var info = pool.getType(sym, index)
+      if (name == nme.CONSTRUCTOR)
+        info match {
+          case MethodType(params, restpe) =>
+            // if this is a non-static inner class, remove the explicit outer parameter
+            val paramsNoOuter = innerClasses getEntry currentClass match {
+              case Some(entry) if !entry.jflags.isStatic =>
+                /* About `clazz.owner.hasPackageFlag` below: scala/bug#5957
+                 * For every nested java class A$B, there are two symbols in the scala compiler.
+                 *  1. created by SymbolLoader, because of the existence of the A$B.class file, owner: package
+                 *  2. created by ClassfileParser of A when reading the inner classes, owner: A
+                 * If symbol 1 gets completed (e.g. because the compiled source mentions `A$B`, not `A#B`), the
+                 * ClassfileParser for 1 executes, and clazz.owner is the package.
+                 */
+                assert(params.head.tpe.typeSymbol == clazz.owner || clazz.owner.hasPackageFlag, params.head.tpe.typeSymbol + ": " + clazz.owner)
+                params.tail
+              case _ =>
+                params
+            }
+            val newParams = paramsNoOuter match {
+              case (init :+ tail) if jflags.isSynthetic =>
+                // scala/bug#7455 strip trailing dummy argument ("access constructor tag") from synthetic constructors which
+                // are added when an inner class needs to access a private constructor.
+                init
+              case _ =>
+                paramsNoOuter
+            }
+
+            info = MethodType(newParams, clazz.tpe)
+        }
+      // Note: the info may be overwritten later with a generic signature
+      // parsed from SignatureATTR
+      sym.setInfo(if (jflags.isVarargs) arrayToRepeated(info) else info)
+      addParamNames(sym)
+    }
+  }
+
+  private def addParamNames(sym: Symbol): Unit = {
+    sym.getAndRemoveAttachment[ParamNames] match {
+      case Some(ParamNames(paramNames)) =>
+        if (sym.hasRawInfo && sym.isMethod) {
+          val params = sym.rawInfo.params
+          val paramNames1 = paramNames.takeRight(params.length)
+          (paramNames1 zip params).foreach {
+            case (nme.NO_NAME, _) => // param was ACC_SYNTHETIC; ignore
+            case (name, param) =>
+              param.resetFlag(SYNTHETIC)
+              param.name = name
+          }
+          if (isDeveloper && !sameLength(paramNames.toList, params)) {
+            // there's not anything we can do, but it's slightly worrisome
+            devWarning(
+              sm"""MethodParameters length mismatch while parsing $sym:
+                  |  rawInfo.params: ${sym.rawInfo.params}
+                  |  MethodParameters: ${paramNames.toList}""")
+          }
+        }
+      case _ =>
+    }
+  }
+
+  final class JavaGenericSignatureTypeCompleter(sig: String, isVarargs: Boolean) extends LazyType {
+    override def complete(sym: symbolTable.Symbol): Unit = {
+      var info = sigToType(sym, sig)
+      sym.setInfo(if (isVarargs) arrayToRepeated(info) else info)
+      addParamNames(sym)
+    }
+  }
 
   def skipAttributes(): Unit = {
     var attrCount: Int = u2
@@ -1340,4 +1358,7 @@ abstract class ClassfileParser {
 
   protected def getScope(flags: JavaAccFlags): Scope =
     if (flags.isStatic) staticScope else instanceScope
+
+  private case class ParamNames(names: Seq[Name])
 }
+

src/reflect/scala/reflect/internal/Types.scala

@@ -1554,53 +1554,9 @@ trait Types
       throw new TypeError("illegal cyclic inheritance involving " + tpe.typeSymbol)
   }
 
-  object baseClassesCycleMonitor {
-    private var open: List[Symbol] = Nil
-    @inline private def cycleLog(msg: => String) {
-      if (settings.debug)
-        Console.err.println(msg)
-    }
-    def size = open.size
-    def push(clazz: Symbol) {
-      cycleLog("+ " + ("  " * size) + clazz.fullNameString)
-      open ::= clazz
-    }
-    def pop(clazz: Symbol) {
-      assert(open.head eq clazz, (clazz, open))
-      open = open.tail
-    }
-    def isOpen(clazz: Symbol) = open contains clazz
-  }
-
   protected def defineBaseClassesOfCompoundType(tpe: CompoundType) {
-    def define() = defineBaseClassesOfCompoundType(tpe, force = false)
-    if (!breakCycles || isPastTyper) define()
-    else tpe match {
-      // non-empty parents helpfully excludes all package classes
-      case tpe @ ClassInfoType(_ :: _, _, clazz) if !clazz.isAnonOrRefinementClass =>
-        // Cycle: force update
-        if (baseClassesCycleMonitor isOpen clazz)
-          defineBaseClassesOfCompoundType(tpe, force = true)
-        else {
-          baseClassesCycleMonitor push clazz
-          try define()
-          finally baseClassesCycleMonitor pop clazz
-        }
-      case _ =>
-        define()
-    }
-  }
-  private def defineBaseClassesOfCompoundType(tpe: CompoundType, force: Boolean) {
     val period = tpe.baseClassesPeriod
-    if (period == currentPeriod) {
-      if (force && breakCycles) {
-        def what = tpe.typeSymbol + " in " + tpe.typeSymbol.owner.fullNameString
-        val bcs  = computeBaseClasses(tpe)
-        tpe.baseClassesCache = bcs
-        warning(s"Breaking cycle in base class computation of $what ($bcs)")
-      }
-    }
-    else {
+    if (period != currentPeriod) {
       tpe.baseClassesPeriod = currentPeriod
       if (!isValidForBaseClasses(period)) {
         val start = if (StatisticsStatics.areSomeColdStatsEnabled) statistics.pushTimer(typeOpsStack, baseClassesNanos) else null

src/reflect/scala/reflect/runtime/JavaUniverseForce.scala

@@ -150,7 +150,6 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     this.SuperType
     this.TypeBounds
     this.CompoundType
-    this.baseClassesCycleMonitor
     this.RefinedType
     this.ClassInfoType
     this.ConstantType

test/files/pos/cycle-jsoup.flags

@@ -23,8 +23,8 @@ object Test extends DirectTest {
       clazz = compiler.rootMirror.staticClass(name)
       constr <- clazz.info.member(termNames.CONSTRUCTOR).alternatives
     } {
-      println(constr.defString)
       fullyInitializeSymbol(constr)
+      println(constr.defString)
     }
   }
 }