Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Constant inlining seems to hide class initialization related issues #7174

Closed
scabug opened this issue Feb 23, 2013 · 4 comments
Closed

Constant inlining seems to hide class initialization related issues #7174

scabug opened this issue Feb 23, 2013 · 4 comments
Assignees
Labels
Milestone

Comments

@scabug
Copy link

scabug commented Feb 23, 2013

Removing Scala's the code in Typers which does constant inlining ...

        case ct @ ConstantType(value) if mode.inNone(TYPEmode | FUNmode) && (ct <:< pt) && !forScaladoc && !forInteractive => // (0)
          val sym = tree.symbol
          if (sym != null && sym.isDeprecated) {
            val msg = sym.toString + sym.locationString + " is deprecated: " + sym.deprecationMessage.getOrElse("")
            unit.deprecationWarning(tree.pos, msg)
          }
          treeCopy.Literal(tree, value)

... seems to have uncovered a NPE ...

quick.plugins:
    [mkdir] Created dir: /home/soc/Entwicklung/scala/build/quick/classes/continuations-plugin
[scalacfork] Compiling 5 files to /home/soc/Entwicklung/scala/build/quick/classes/continuations-plugin
[scalacfork] error: java.lang.NullPointerException
[scalacfork] 	at scala.tools.nsc.SubComponent.<init>(SubComponent.scala:48)
[scalacfork] 	at scala.tools.nsc.typechecker.Analyzer$namerFactory$.<init>(Analyzer.scala:32)
[scalacfork] 	at scala.tools.nsc.Global$$anon$1.namerFactory$lzycompute(Global.scala:439)
[scalacfork] 	at scala.tools.nsc.Global$$anon$1.namerFactory(Global.scala:439)
[scalacfork] 	at scala.tools.nsc.Global.computeInternalPhases(Global.scala:653)
[scalacfork] 	at scala.tools.nsc.Global.computePhaseDescriptors(Global.scala:695)
[scalacfork] 	at scala.tools.nsc.Global.phaseDescriptors$lzycompute(Global.scala:702)
[scalacfork] 	at scala.tools.nsc.Global.phaseDescriptors(Global.scala:702)
[scalacfork] 	at scala.tools.nsc.Global$Run.<init>(Global.scala:1210)
[scalacfork] 	at scala.tools.nsc.Driver.doCompile(Driver.scala:32)
[scalacfork] 	at scala.tools.nsc.Main$.doCompile(Main.scala:53)
[scalacfork] 	at scala.tools.nsc.Driver.process(Driver.scala:54)
[scalacfork] 	at scala.tools.nsc.Driver.main(Driver.scala:67)
[scalacfork] 	at scala.tools.nsc.Main.main(Main.scala)
[scalacfork] 
[scalacfork] Exception in thread "main" java.lang.NullPointerException
[scalacfork] 	at scala.tools.nsc.SubComponent.<init>(SubComponent.scala:48)
[scalacfork] 	at scala.tools.nsc.typechecker.Analyzer$namerFactory$.<init>(Analyzer.scala:32)
[scalacfork] 	at scala.tools.nsc.Global$$anon$1.namerFactory$lzycompute(Global.scala:439)
[scalacfork] 	at scala.tools.nsc.Global$$anon$1.namerFactory(Global.scala:439)
[scalacfork] 	at scala.tools.nsc.Global.computeInternalPhases(Global.scala:653)
[scalacfork] 	at scala.tools.nsc.Global.computePhaseDescriptors(Global.scala:695)
[scalacfork] 	at scala.tools.nsc.Global.phaseDescriptors$lzycompute(Global.scala:702)
[scalacfork] 	at scala.tools.nsc.Global.phaseDescriptors(Global.scala:702)
[scalacfork] 	at scala.tools.nsc.Global$Run.<init>(Global.scala:1210)
[scalacfork] 	at scala.tools.nsc.Driver.doCompile(Driver.scala:32)
[scalacfork] 	at scala.tools.nsc.Main$.doCompile(Main.scala:53)
[scalacfork] 	at scala.tools.nsc.Driver.process(Driver.scala:54)
[scalacfork] 	at scala.tools.nsc.Driver.main(Driver.scala:67)
[scalacfork] 	at scala.tools.nsc.Main.main(Main.scala)

... which I think is an issue with class initialization of code related to SubComponent/Global.

Fixing this would probably allow us to get rid of constant inlining, which in turn would simplify code doing dependency tracking and incremental compilation and reduce the working set by not having to recompile all clients when the variable is changed.

I pushed the commit to https://github.com/soc/scala/commits/poc/no-constant-inlining
SHA1 is de4d854d5a2b9782d3a82e7aa94987f1eae40acf

Background: Constant inlining was introduced in Java in an misguided attempt to support conditional compilation. What it basically does is to replace references to “constant variables”¹ with the value itself, leading to tons of subtle bugs if values change. The behaviour has been called a mistake and the “chink in the armor of dynamic linking of libraries” by the designers of Java itself.

¹ Constant variables are primitive values, but not their wrappers; Strings, but no Enums or nulls. (Consistency is great isn't it?)

@scabug
Copy link
Author

scabug commented Feb 23, 2013

Imported From: https://issues.scala-lang.org/browse/SI-7174?orig=1
Reporter: @soc
Affected Versions: 2.11.0-M1

@scabug
Copy link
Author

scabug commented Jul 1, 2013

@gkossakowski said:
Here's minimal reproduction of the issue:

scala> :paste
// Entering paste mode (ctrl-D to finish)

abstract class Global {
  final val NoRunId = 14
}

abstract class Foo {
  val global: Global
  val ownPhaseRunId = global.NoRunId
}
new Foo { val global = null }

// Exiting paste mode, now interpreting.

defined class Global
defined class Foo
res0: Foo{val global: Null} = $anon$1@9273085

scala> res0.ownPhaseRunId
res1: Int = 14

We inline the constant and thus never refer to global so it doesn't blow up with NPE. What's worse, -Xcheckinit won't help you because it runs after the constant has been inlined and there's no sign of accessing global anymore.

See also: https://groups.google.com/d/topic/scala-internals/L_8q9tv1uTI/discussion (where SubComponent case is discussed).

@scabug
Copy link
Author

scabug commented Jul 12, 2013

@adriaanm said:
Thanks, Simon!

@scabug scabug closed this as completed Jul 12, 2013
@scabug
Copy link
Author

scabug commented Jul 12, 2013

@adriaanm said:
scala/scala#2708

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants