Skip to content
This repository
Browse code

SI-7251, compiler crash with $.

We don't need to assert our way out of tight spots, we can issue
an error. Or so I once thought.

It turns out lots of assertions have been disappearing before
being heard thanks to "case t: Throwable". Under such conditions,
a failed assertion is a no-op, but an error is an error.

The crash associated with SI-7251 is best avoided by removing the
assertion, which allows an error to be issued in the normal course
of events.

In the course of trying to figure out the above, I cleaned up
ClassfileParser somewhat.
  • Loading branch information...
commit 395e90a786874ebe795e656f532b50110c8c1a98 1 parent b7b4f87
Paul Phillips authored March 15, 2013
241  src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
@@ -44,7 +44,7 @@ abstract class ClassfileParser {
44 44
 
45 45
   def srcfile = srcfile0
46 46
 
47  
-  private def currentIsTopLevel = currentClass.toString.indexOf('$') < 0
  47
+  private def currentIsTopLevel = !(currentClass.decodedName containsChar '$')
48 48
 
49 49
   private object unpickler extends scala.reflect.internal.pickling.UnPickler {
50 50
     val global: ClassfileParser.this.global.type = ClassfileParser.this.global
@@ -436,63 +436,58 @@ abstract class ClassfileParser {
436 436
     sym
437 437
   }
438 438
 
439  
-  /** Return the class symbol of the given name. */
440  
-  def classNameToSymbol(name: Name): Symbol = {
441  
-    def loadClassSymbol(name: Name): Symbol = {
442  
-      val file = global.classPath findSourceFile ("" +name) getOrElse {
443  
-        // SI-5593 Scaladoc's current strategy is to visit all packages in search of user code that can be documented
444  
-        // therefore, it will rummage through the classpath triggering errors whenever it encounters package objects
445  
-        // that are not in their correct place (see bug for details)
446  
-        if (!settings.isScaladoc)
447  
-          warning("Class " + name + " not found - continuing with a stub.")
448  
-        return NoSymbol.newClass(name.toTypeName)
449  
-      }
450  
-      val completer     = new global.loaders.ClassfileLoader(file)
451  
-      var owner: Symbol = rootMirror.RootClass
452  
-      var sym: Symbol   = NoSymbol
453  
-      var ss: Name      = null
454  
-      var start         = 0
455  
-      var end           = name indexOf '.'
456  
-
457  
-      while (end > 0) {
458  
-        ss = name.subName(start, end)
459  
-        sym = owner.info.decls lookup ss
460  
-        if (sym == NoSymbol) {
461  
-          sym = owner.newPackage(ss) setInfo completer
462  
-          sym.moduleClass setInfo completer
463  
-          owner.info.decls enter sym
464  
-        }
465  
-        owner = sym.moduleClass
466  
-        start = end + 1
467  
-        end = name.indexOf('.', start)
468  
-      }
469  
-      ss = name.subName(0, start)
470  
-      owner.info.decls lookup ss orElse {
471  
-        sym = owner.newClass(ss.toTypeName) setInfoAndEnter completer
472  
-        debuglog("loaded "+sym+" from file "+file)
473  
-        sym
  439
+  private def loadClassSymbol(name: Name): Symbol = {
  440
+    val file = global.classPath findSourceFile ("" +name) getOrElse {
  441
+      // SI-5593 Scaladoc's current strategy is to visit all packages in search of user code that can be documented
  442
+      // therefore, it will rummage through the classpath triggering errors whenever it encounters package objects
  443
+      // that are not in their correct place (see bug for details)
  444
+      if (!settings.isScaladoc)
  445
+        warning(s"Class $name not found - continuing with a stub.")
  446
+      return NoSymbol.newClass(name.toTypeName)
  447
+    }
  448
+    val completer     = new global.loaders.ClassfileLoader(file)
  449
+    var owner: Symbol = rootMirror.RootClass
  450
+    var sym: Symbol   = NoSymbol
  451
+    var ss: Name      = null
  452
+    var start         = 0
  453
+    var end           = name indexOf '.'
  454
+
  455
+    while (end > 0) {
  456
+      ss = name.subName(start, end)
  457
+      sym = owner.info.decls lookup ss
  458
+      if (sym == NoSymbol) {
  459
+        sym = owner.newPackage(ss) setInfo completer
  460
+        sym.moduleClass setInfo completer
  461
+        owner.info.decls enter sym
474 462
       }
  463
+      owner = sym.moduleClass
  464
+      start = end + 1
  465
+      end = name.indexOf('.', start)
475 466
     }
476  
-
477  
-    def lookupClass(name: Name) = try {
478  
-      if (name.pos('.') == name.length)
479  
-        definitions.getMember(rootMirror.EmptyPackageClass, name.toTypeName)
480  
-      else
481  
-        rootMirror.getClass(name) // see tickets #2464, #3756
482  
-    } catch {
483  
-      case _: FatalError => loadClassSymbol(name)
  467
+    ss = name.subName(0, start)
  468
+    owner.info.decls lookup ss orElse {
  469
+      sym = owner.newClass(ss.toTypeName) setInfoAndEnter completer
  470
+      debuglog("loaded "+sym+" from file "+file)
  471
+      sym
484 472
     }
  473
+  }
  474
+  /** FIXME - we shouldn't be doing ad hoc lookups in the empty package.
  475
+   *  The method called "getClassByName" should either return the class or not.
  476
+   */
  477
+  private def lookupClass(name: Name) = (
  478
+    if (name containsChar '.')
  479
+      rootMirror getClassByName name // see tickets #2464, #3756
  480
+    else
  481
+      definitions.getMember(rootMirror.EmptyPackageClass, name.toTypeName)
  482
+  )
485 483
 
486  
-    innerClasses.get(name) match {
487  
-      case Some(entry) =>
488  
-        //println("found inner class " + name)
489  
-        val res = innerClasses.classSymbol(entry.externalName)
490  
-        //println("\trouted to: " + res)
491  
-        res
492  
-      case None =>
493  
-        //if (name.toString.contains("$")) println("No inner class: " + name + innerClasses + " while parsing " + in.file.name)
494  
-        lookupClass(name)
495  
-    }
  484
+  /** Return the class symbol of the given name. */
  485
+  def classNameToSymbol(name: Name): Symbol = {
  486
+    if (innerClasses contains name)
  487
+      innerClasses innerSymbol name
  488
+    else
  489
+      try lookupClass(name)
  490
+      catch { case _: FatalError => loadClassSymbol(name) }
496 491
   }
497 492
 
498 493
   var sawPrivateConstructor = false
@@ -646,7 +641,7 @@ abstract class ClassfileParser {
646 641
           info match {
647 642
             case MethodType(params, restpe) =>
648 643
               // if this is a non-static inner class, remove the explicit outer parameter
649  
-              val newParams = innerClasses.get(currentClass) match {
  644
+              val newParams = innerClasses getEntry currentClass match {
650 645
                 case Some(entry) if !isScalaRaw && !isStatic(entry.jflags) =>
651 646
                   /* About `clazz.owner.isPackage` below: SI-5957
652 647
                    * For every nested java class A$B, there are two symbols in the scala compiler.
@@ -748,7 +743,7 @@ abstract class ClassfileParser {
748 743
                 res
749 744
               }
750 745
             case tp =>
751  
-              assert(sig.charAt(index) != '<', tp)
  746
+              assert(sig.charAt(index) != '<', s"sig=$sig, index=$index, tp=$tp")
752 747
               tp
753 748
           }
754 749
 
@@ -1107,7 +1102,7 @@ abstract class ClassfileParser {
1107 1102
       unlinkIfPresent(cName.toTypeName)
1108 1103
     }
1109 1104
 
1110  
-    for (entry <- innerClasses.values) {
  1105
+    for (entry <- innerClasses.entries) {
1111 1106
       // create a new class member for immediate inner classes
1112 1107
       if (entry.outerName == currentClass) {
1113 1108
         val file = global.classPath.findSourceFile(entry.externalName.toString) getOrElse {
@@ -1145,14 +1140,9 @@ abstract class ClassfileParser {
1145 1140
         case tpnme.InnerClassesATTR if !isScala =>
1146 1141
           val entries = in.nextChar.toInt
1147 1142
           for (i <- 0 until entries) {
1148  
-            val innerIndex = in.nextChar.toInt
1149  
-            val outerIndex = in.nextChar.toInt
1150  
-            val nameIndex = in.nextChar.toInt
1151  
-            val jflags = in.nextChar.toInt
1152  
-            if (innerIndex != 0 && outerIndex != 0 && nameIndex != 0) {
1153  
-              val entry = InnerClassEntry(innerIndex, outerIndex, nameIndex, jflags)
1154  
-              innerClasses += (pool.getClassName(innerIndex) -> entry)
1155  
-            }
  1143
+            val innerIndex, outerIndex, nameIndex, jflags = in.nextChar.toInt
  1144
+            if (innerIndex != 0 && outerIndex != 0 && nameIndex != 0)
  1145
+              innerClasses add InnerClassEntry(innerIndex, outerIndex, nameIndex, jflags)
1156 1146
           }
1157 1147
         case _ =>
1158 1148
           in.skip(attrLen)
@@ -1166,72 +1156,69 @@ abstract class ClassfileParser {
1166 1156
     def externalName = pool getClassName external
1167 1157
     def outerName    = pool getClassName outer
1168 1158
     def originalName = pool getName name
  1159
+    def isStatic     = ClassfileParser.this.isStatic(jflags)
  1160
+    def isModule     = originalName.isTermName
  1161
+    def scope        = if (isStatic) staticScope else instanceScope
  1162
+    def enclosing    = if (isStatic) enclModule else enclClass
  1163
+
  1164
+    // The name of the outer class, without its trailing $ if it has one.
  1165
+    private def strippedOuter = nme stripModuleSuffix outerName
  1166
+    private def isInner       = innerClasses contains strippedOuter
  1167
+    private def enclClass     = if (isInner) innerClasses innerSymbol strippedOuter else classNameToSymbol(strippedOuter)
  1168
+    private def enclModule    = enclClass.companionModule
  1169
+
  1170
+    private def staticWord = if (isStatic) "static " else ""
  1171
+    override def toString = s"$staticWord$originalName in $outerName ($externalName)"
  1172
+  }
1169 1173
 
1170  
-    override def toString =
1171  
-      originalName + " in " + outerName + "(" + externalName +")"
  1174
+  /** Return the Symbol of the top level class enclosing `name`,
  1175
+   *  or the symbol of `name` itself if no enclosing classes are found.
  1176
+   */
  1177
+  def topLevelClass(name: Name): Symbol = innerClasses getEntry name match {
  1178
+    case Some(entry) => topLevelClass(entry.outerName)
  1179
+    case _           => classNameToSymbol(name)
1172 1180
   }
1173 1181
 
1174  
-  object innerClasses extends scala.collection.mutable.HashMap[Name, InnerClassEntry] {
1175  
-    /** Return the Symbol of the top level class enclosing `name`,
1176  
-     *  or 'name's symbol if no entry found for `name`.
1177  
-     */
1178  
-    def topLevelClass(name: Name): Symbol = {
1179  
-      val tlName = if (isDefinedAt(name)) {
1180  
-        var entry = this(name)
1181  
-        while (isDefinedAt(entry.outerName))
1182  
-          entry = this(entry.outerName)
1183  
-        entry.outerName
1184  
-      } else
1185  
-        name
1186  
-      classNameToSymbol(tlName)
  1182
+  /** Return the class symbol for the given name. It looks it up in its outer class.
  1183
+   *  Forces all outer class symbols to be completed.
  1184
+   *
  1185
+   *  If the given name is not an inner class, it returns the symbol found in `definitions`.
  1186
+   */
  1187
+  object innerClasses {
  1188
+    private val inners = mutable.HashMap[Name, InnerClassEntry]()
  1189
+
  1190
+    def contains(name: Name) = inners contains name
  1191
+    def getEntry(name: Name) = inners get name
  1192
+    def entries              = inners.values
  1193
+
  1194
+    def add(entry: InnerClassEntry): Unit = {
  1195
+      inners get entry.externalName foreach (existing =>
  1196
+        debugwarn(s"Overwriting inner class entry! Was $existing, now $entry")
  1197
+      )
  1198
+      inners(entry.externalName) = entry
1187 1199
     }
1188  
-
1189  
-    /** Return the class symbol for `externalName`. It looks it up in its outer class.
1190  
-     *  Forces all outer class symbols to be completed.
1191  
-     *
1192  
-     *  If the given name is not an inner class, it returns the symbol found in `definitions`.
1193  
-     */
1194  
-    def classSymbol(externalName: Name): Symbol = {
1195  
-      /** Return the symbol of `innerName`, having the given `externalName`. */
1196  
-      def innerSymbol(externalName: Name, innerName: Name, static: Boolean): Symbol = {
1197  
-        def getMember(sym: Symbol, name: Name): Symbol =
1198  
-          if (static)
1199  
-            if (sym == clazz) staticScope.lookup(name)
1200  
-            else sym.companionModule.info.member(name)
1201  
-          else
1202  
-            if (sym == clazz) instanceScope.lookup(name)
1203  
-            else sym.info.member(name)
1204  
-
1205  
-        innerClasses.get(externalName) match {
1206  
-          case Some(entry) =>
1207  
-            val outerName = nme.stripModuleSuffix(entry.outerName)
1208  
-            val sym = classSymbol(outerName)
1209  
-            val s =
1210  
-              // if loading during initialization of `definitions` typerPhase is not yet set.
1211  
-              // in that case we simply load the member at the current phase
1212  
-              if (currentRun.typerPhase != null)
1213  
-                beforeTyper(getMember(sym, innerName.toTypeName))
1214  
-              else
1215  
-                getMember(sym, innerName.toTypeName)
1216  
-
1217  
-            assert(s ne NoSymbol,
1218  
-              "" + ((externalName, outerName, innerName, sym.fullLocationString)) + " / " +
1219  
-              " while parsing " + ((in.file, busy)) +
1220  
-              sym + "." + innerName + " linkedModule: " + sym.companionModule + sym.companionModule.info.members
1221  
-            )
1222  
-            s
1223  
-
1224  
-          case None =>
1225  
-            classNameToSymbol(externalName)
1226  
-        }
1227  
-      }
1228  
-
1229  
-      get(externalName) match {
1230  
-        case Some(entry) =>
1231  
-          innerSymbol(entry.externalName, entry.originalName, isStatic(entry.jflags))
1232  
-        case None =>
1233  
-          classNameToSymbol(externalName)
1234  
-      }
  1200
+    def innerSymbol(externalName: Name): Symbol = this getEntry externalName match {
  1201
+      case Some(entry) => innerSymbol(entry)
  1202
+      case _           => NoSymbol
  1203
+    }
  1204
+    // if loading during initialization of `definitions` typerPhase is not yet set.
  1205
+    // in that case we simply load the member at the current phase
  1206
+    @inline private def enteringTyperIfPossible(body: => Symbol): Symbol =
  1207
+      if (currentRun.typerPhase eq null) body else beforeTyper(body)
  1208
+
  1209
+    private def innerSymbol(entry: InnerClassEntry): Symbol = {
  1210
+      val name      = entry.originalName.toTypeName
  1211
+      val enclosing = entry.enclosing
  1212
+      def getMember = (
  1213
+        if (enclosing == clazz) entry.scope lookup name
  1214
+        else enclosing.info member name
  1215
+      )
  1216
+      enteringTyperIfPossible(getMember)
  1217
+      /** There used to be an assertion that this result is not NoSymbol; changing it to an error
  1218
+       *  revealed it had been going off all the time, but has been swallowed by a catch t: Throwable
  1219
+       *  in Repository.scala. Since it has been accomplishing nothing except misleading anyone who
  1220
+       *  thought it wasn't triggering, I removed it entirely.
  1221
+       */
1235 1222
     }
1236 1223
   }
1237 1224
 
4  test/files/neg/t7251.check
... ...
@@ -0,0 +1,4 @@
  1
+B_2.scala:5: error: object s.Outer$Triple$ is not a value
  2
+    println( s.Outer$Triple$ )
  3
+               ^
  4
+one error found
10  test/files/neg/t7251/A_1.scala
... ...
@@ -0,0 +1,10 @@
  1
+package s
  2
+
  3
+object Outer {
  4
+  type Triple[+A, +B, +C] = Tuple3[A, B, C]
  5
+  object Triple {
  6
+    def apply[A, B, C](x: A, y: B, z: C) = Tuple3(x, y, z)
  7
+    def unapply[A, B, C](x: Tuple3[A, B, C]): Option[Tuple3[A, B, C]] = Some(x)
  8
+  }
  9
+}
  10
+
7  test/files/neg/t7251/B_2.scala
... ...
@@ -0,0 +1,7 @@
  1
+package s
  2
+
  3
+object Test {
  4
+  def main(args: Array[String]): Unit = {
  5
+    println( s.Outer$Triple$ )
  6
+  }
  7
+}

0 notes on commit 395e90a

Please sign in to comment.
Something went wrong with that request. Please try again.