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

Fix #1441: init MODULE$ in <clinit> #9181

Merged
merged 35 commits into from
Aug 7, 2020
Merged

Conversation

liufengyun
Copy link
Contributor

@liufengyun liufengyun commented Jun 15, 2020

Fix #1441: init $MODULE in <clinit>

Related: #5928 #8652

@liufengyun liufengyun changed the title Fix #1441: init $MODULE in <clinit> Fix #1441: init MODULE$ in <clinit> Jun 15, 2020
@liufengyun liufengyun force-pushed the fix-1441b branch 2 times, most recently from c86ca4d to 49d88a7 Compare July 10, 2020 14:47
@liufengyun liufengyun force-pushed the fix-1441b branch 2 times, most recently from 37fac7f to 6ca1b10 Compare July 15, 2020 22:03
@liufengyun liufengyun marked this pull request as ready for review July 16, 2020 05:38
@liufengyun liufengyun requested a review from smarter July 16, 2020 10:21
@odersky
Copy link
Contributor

odersky commented Jul 17, 2020

Good work! For generating super accessors you can use AccessProxies. It already contains most of the logic to produce protected and inline accessors, which can be shared.

@liufengyun liufengyun force-pushed the fix-1441b branch 3 times, most recently from 7c768a3 to 475ee8b Compare July 21, 2020 23:23
@smarter
Copy link
Member

smarter commented Jul 23, 2020

test performance please

@dottybot
Copy link
Member

performance test scheduled: 1 job(s) in queue, 0 running.

@dottybot
Copy link
Member

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/9181/ to see the changes.

Benchmarks is based on merging with master (0f1a23e)

liufengyun and others added 22 commits August 6, 2020 22:15
This is a port of the following PR in Scala 2:

scala/scala#3935
Otherwise, we will need to support trees like
`C.this.$outer.super[D]` as in Scala 2,
which complicates invariants about super.
Co-authored-by: Guillaume Martres <smarter@ubuntu.com>
We should check the type instead,
which is more semantic.
Still we have a problem with the following magic:

https://github.com/scala/scala/pull/7270/files#r221195225

It is a magic for two reasons:

1. It generates `getstatic` for `Select(This(O$), x)`
2. It is not affected by compilation order of classes

For the second, suppose the backend compiles `A$B` before `A`.
Then the class `A$B` does not see that `x` is static,
which should cause problem at runtime:

```
object A {
  private[this] val x: Int = 10

  class B {
    val y = x
  }
}
```

The following are the reasons why it works in scalac:

1. inner classes always come after the outer class
2. non `private[this]` fields are accessed via accessor methods, which are not static
3. inside the module class, access to the fields are compiled after setting the static flag
4. the code generation specialize for trees like `Select(This(O$), x)`, where `x` is a static member
The magic in scalac is order-dependent:

https://github.com/scala/scala/pull/7270/files#r221195225

See the previous commit for more detailed info.
There are compilation errors due to changes between 8 and 11

- addition of `String.lines` shadows the deprecated `StringOps.lines` in stdlib
  * probblem for ScalaTest
- JAXB APIs are removed
  * problem for BetterFiles
@smarter
Copy link
Member

smarter commented Aug 6, 2020

test performance please

@dottybot
Copy link
Member

dottybot commented Aug 6, 2020

performance test scheduled: 1 job(s) in queue, 1 running.

@dottybot
Copy link
Member

dottybot commented Aug 6, 2020

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/9181/ to see the changes.

Benchmarks is based on merging with master (b8825a2)

@liufengyun liufengyun merged commit d5bdea8 into scala:master Aug 7, 2020
@liufengyun liufengyun deleted the fix-1441b branch August 7, 2020 07:47
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 this pull request may close these issues.

JDK9 blocker: stops supporting our scheme for MODULE$ initialization
6 participants