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

JDK9 blocker: stops supporting our scheme for MODULE$ initialization #1441

Closed
DarkDimius opened this issue Aug 3, 2016 · 12 comments · Fixed by #9181
Closed

JDK9 blocker: stops supporting our scheme for MODULE$ initialization #1441

DarkDimius opened this issue Aug 3, 2016 · 12 comments · Fixed by #9181
Assignees

Comments

@DarkDimius
Copy link
Contributor

DarkDimius commented Aug 3, 2016

https://bugs.openjdk.java.net/browse/JDK-8159215

the way we currently compile module classes, assigns the static final MODULE$ from the module class constructor <init> which is called by <clinit>. This is prohibited by the spec and will be enforced by JDK9.

./scala-2.11.8/bin/scala
Exception in thread "main" java.lang.IllegalAccessError: Update to static final field scala.tools.nsc.MainGenericRunner$.MODULE$ attempted from a different method (scala.tools.nsc.MainGenericRunner$) than the initializer method <clinit>
        at scala.tools.nsc.MainGenericRunner$.<init>(MainGenericRunner.scala:102)
        at scala.tools.nsc.MainGenericRunner$.<clinit>(MainGenericRunner.scala)
        at scala.tools.nsc.MainGenericRunner.main(MainGenericRunner.scala)

If they enforce the restriction and we move the initializers to the only allowed place, <clinit>, following code will always throw NPE.

class Super {
  Object.foo
}

object Singleton extends Super {

 def foo = 1

 // <clinit> -> new Singleton
 // <init> MODULE$ = this
}

Reported by Zoltán Majó. We have discussed the issue. The JDK product team seems have decided that this change is to stay. What it means, is that code compiled both by dotty and Scala cannot use bytecode version 53, unless we drop the final flag on all MODULE$.

PING @adriaanm, @lrytz, @retronym. I'll keep you informed on progress.

@DarkDimius
Copy link
Contributor Author

I have an idea how to fix this without breaking current behavior.
Will need to check when I'm back.

@retronym
Copy link
Member

retronym commented Aug 4, 2016

@DarkDimius Is this what you had in mind? scala/scala@2.12.x...retronym:topic/module-init

@DarkDimius
Copy link
Contributor Author

looks like it :+1

On 4 Aug 2016, at 12:13, Jason Zaugg wrote:

@DarkDimius Is this what you had in mind?
scala/scala@2.12.x...retronym:topic/module-init


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#1441 (comment)

@DarkDimius
Copy link
Contributor Author

DarkDimius commented Aug 5, 2016

@retronym, there's one piece missing in your change. You need to move the body of constructor to the static initializer. Here's an example that demostrates the need:

scala> class A { def foo = O.bar}; object O extends A {foo; def bar = println("bar")}
defined class A
defined object O

scala> O
bar
res1: O.type = O$@1593948d

By the time user-defined body of constructor of objects starts execution, the MODULE$ should be published.

@smarter
Copy link
Member

smarter commented Aug 5, 2016

From the bug report you linked, it looks like they want to enforce this check for older classfiles too:

In the case of final fields, an old class file that assigns to one outside the appropriate initialization method is very much in the wrong, so gating the putfield/putstatic logic by class file version is unnecessary.

Should someone comment on that bug report to let them know that doing this would break every existing Scala classfile?

@DarkDimius
Copy link
Contributor Author

DarkDimius commented Aug 5, 2016

@smarter they are not enforcing this for old classfiles. I'm in contact with them.

@retronym
Copy link
Member

retronym commented Aug 5, 2016

I believe that it's preferable to expose the null that to expose a partially initialised object. That's the status quo, no?

@DarkDimius
Copy link
Contributor Author

@retronym current behavior publishes a partially initialized object that other classes can use(see the example above). If we prohibit this, it would mean that in the body of the object you cannot use anything that transitively uses the same object. I believe this is a very harsh restriction that is hard to maintain in big codebases.

@DarkDimius
Copy link
Contributor Author

another example, that does not involve superclasses:

scala> class CC { def foo = O.bar}; object O {new CC().foo; def bar = println(1)};
defined class CC
defined object O

scala> O
1
res7: O.type = O$@36ebc363

@liufengyun
Copy link
Contributor

@smarter Is this issue still relevant, given Won't Fix for https://bugs.openjdk.java.net/browse/JDK-8159215 ?

@smarter
Copy link
Member

smarter commented Mar 1, 2019

Is this issue still relevant ?

Yes, the won't fix is about adding the check when the JVM load classfiles where the Java bytecode version corresponds to Java <= 8, but the issue remains for newer Java bytecode versions, we need to implement the same scheme that 2.13 has implemented: scala/scala#7270

@liufengyun
Copy link
Contributor

This one depends on #9099 in order to construct trees.

liufengyun added a commit to dotty-staging/dotty that referenced this issue Jun 15, 2020
This is a port of the following PR from Scala 2:
scala/scala#7270
liufengyun added a commit to dotty-staging/dotty that referenced this issue Jun 15, 2020
This is a port of the following PR from Scala 2:
scala/scala#7270
liufengyun added a commit to dotty-staging/dotty that referenced this issue Jul 10, 2020
This is a port of the following PR from Scala 2:
scala/scala#7270
liufengyun added a commit to dotty-staging/dotty that referenced this issue Jul 15, 2020
This is a port of the following PR from Scala 2:
scala/scala#7270
liufengyun added a commit to dotty-staging/dotty that referenced this issue Jul 21, 2020
This is a port of the following PR from Scala 2:
scala/scala#7270
liufengyun added a commit to dotty-staging/dotty that referenced this issue Jul 22, 2020
This is a port of the following PR from Scala 2:
scala/scala#7270
liufengyun added a commit to dotty-staging/dotty that referenced this issue Aug 3, 2020
This is a port of the following PR from Scala 2:
scala/scala#7270
liufengyun added a commit to dotty-staging/dotty that referenced this issue Aug 6, 2020
This is a port of the following PR from Scala 2:
scala/scala#7270
liufengyun added a commit that referenced this issue Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants