scala / bug

Scala 2 bug reports only. Please, no questions — proper bug reports only.
https://scala-lang.org
232 stars 21 forks source link

Error in nested object initialization prevents sbt from loading compiler plugins #4426

Closed scabug closed 13 years ago

scabug commented 13 years ago

Sbt doesn't load compiler plugins when using 2.9 RC1 (used to work fine with 2.8). I've narrowed it down a bit and it seems like there's an underlying error in scalac that has to do with nested objects not being initialized correctly. The weird thing about it is that it happens only to the first nested object. In the repl script below, uncommenting the dummy object makes the error go away.

=== What steps will reproduce the problem (please be specific and use wikiformatting)? ===

Welcome to Scala version 2.9.0.RC1 (Java HotSpot(TM) 64-Bit Server VM, Java 1.6.0_24).
Type in expressions to have them evaluated.
Type :help for more information.

scala> import scala.tools.nsc._
import scala.tools.nsc._

scala> :power
** Power User mode enabled - BEEP BOOP WHIR **
** scala.tools.nsc._ has been imported      **
** global._ and definitions._ also imported **
** New vals! Try repl, intp, global, power  **
** New cmds! :help to discover them         **
** New defs! Type power.<tab> to reveal     **

scala> {
     | object cc extends Global(settings) { 
     |   
     |   //object dummy   //<-- uncomment to make the error go away
     |   
     |   object sbtAnalyzer extends {
     |     val global: cc.type = cc
     |     val phaseName = "bla"
     |     val runsAfter = List()
     |     val runsRightAfter = None
     |   } with SubComponent {
     |     def newPhase(prev: Phase) = null.asInstanceOf[Phase]
     |     def name = phaseName
     |   }
     |   
     |   override def computePluginPhases() = {
     |     val r = super.computePluginPhases()
     |     assert(sbtAnalyzer ne null, "FAIL!")
     |     addToPhasesSet(sbtAnalyzer, "sbt analyzer")
     |     r
     |   }
     | }
     | println(new cc.Run)
     | }
java.lang.AssertionError: assertion failed: FAIL!
    at scala.Predef$$.assert(Predef.scala:100)
    at cc$$2$$.computePluginPhases(<console>:44)
    at scala.tools.nsc.Global.computePhaseDescriptors(Global.scala:595)
    at scala.tools.nsc.Global.phaseDescriptors(Global.scala:600)
    at scala.tools.nsc.Global$$Run.<init>(Global.scala:689)
    at .<init>(<console>:49)
    at .<clinit>(<console>)
    at .<init>(<console>:11)
    at .<clinit>(<console>)
    at $$export(<console>)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
    at java.lang.reflect.Method.invoke(Method.java:597)
    at scala.tools.nsc.interpreter.IMain$$ReadEvalPrint.call(IMain.scala:669)
    at scala.tools.nsc.interpreter.IMain$$Request$$$$anonfun$$11.apply(IMain.scala:903)
    at scala.tools.nsc.interpreter.Line$$$$anonfun$$1.apply$$mcV$$sp(Line.scala:43)
    at scala.tools.nsc.io.package$$$$anon$$2.run(package.scala:31)
    at java.lang.Thread.run(Thread.java:680)

scala> 

=== What is the expected behavior? ===

object sbtAnalyzer should be initialized (lazily), no matter whether it is syntactically the first nested object or not.

=== What do you see instead? ===

sbtAnalyzer is null. If there is another object declaration before sbtAnalyzer, it is initialized correctly.

=== What versions of the following are you using? ===

scabug commented 13 years ago

Imported From: https://issues.scala-lang.org/browse/SI-4426?orig=1 Reporter: @TiarkRompf

scabug commented 13 years ago

@paulp said: Here's a somewhat narrowed reproduction which doesn't use the repl. The call to super.computePluginPhases() is necessary for the crash.

import scala.tools.nsc._

object Test {
  val x = {
    object cc extends Global(new Settings()) { 
      // object dummy   //<-- uncomment to make the error go away
      object sbtAnalyzer { }

      override def computePluginPhases() = {
        super.computePluginPhases()
        assert(sbtAnalyzer ne null, "FAIL!")
      }
    }
    println(new cc.Run)
  }

  def main(args: Array[String]): Unit = {

  }
}
scabug commented 13 years ago

@paulp said: See also #4432.

scabug commented 13 years ago

@paulp said: Of unknown relevance, but note this comment I placed at some distantly remembered time in computePluginPhases.

  lazy val plugins: List[Plugin] = loadPlugins
  // ...
  protected def computePluginPhases(): Unit = {
    // For reasons not yet apparent to me, plugins started appearing
    // as null when I added phaseTimings to global.
    if (plugins != null)
      phasesSet ++= (plugins flatMap (_.components))
  }

Since plugins is a lazy val it probably shouldn't be appearing as null. Also note that if I override a different randomly chosen method in similar fashion, it doesn't crash. So this manifestation of the issue may depend on what exactly this method is doing.

scabug commented 13 years ago

@odersky said: Looks like this one is a release blocker.

scabug commented 13 years ago

@hubertp said: Replying to [comment:7 extempore]:

Of unknown relevance, but note this comment I placed at some distantly remembered time in computePluginPhases. {code} lazy val plugins: List[Plugin] = loadPlugins // ... protected def computePluginPhases(): Unit = { // For reasons not yet apparent to me, plugins started appearing // as null when I added phaseTimings to global. if (plugins != null) phasesSet ++= (plugins flatMap (_.components)) } }} Since plugins is a lazy val it probably shouldn't be appearing as null. Also note that if I override a different randomly chosen method in similar fashion, it doesn't crash. So this manifestation of the issue may depend on what exactly this method is doing.

And actually phaseTimings are the real culprit (I mean the private thing is essential) as it messes up the numbering for bitmaps. Should be easy to fix now.

scabug commented 13 years ago

@hubertp said: (In a3b106bf60) closes #4426. name mangling for private lazy vals that are lifted during explicitouter is now taken into account. this also handles lazy vals/objects inside anonymous classes. review by dragos.

scabug commented 13 years ago

@hubertp said: (In 6c1feb586b) remove now redundant check (see #4426). review by extempore