Fix undercompilation when depending on inner class #9694
Merged
+148
−18
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
smarter
force-pushed
the
fix-incremental-inner
branch
from
5e9488e to
bb2dee8
Compare
|
not sure what Is happening, [info] [info] Compiling 1 Scala source to /private/var/folders/m4/4dsrk6zn74v3zzpk34nj11mh0000gn/T/sbt_e6a419e4/inner-class/project/target/scala-2.12/sbt-1.0/classes ...
[info] [info] Done compiling.
[info] [info] loading settings for project inner-class from build.sbt ...
[info] [info] set current project to inner-class (in build file:/private/var/folders/m4/4dsrk6zn74v3zzpk34nj11mh0000gn/T/sbt_e6a419e4/inner-class/)
[info] [info] Compiling 2 Scala sources to /private/var/folders/m4/4dsrk6zn74v3zzpk34nj11mh0000gn/T/sbt_e6a419e4/inner-class/target/scala-0.28/classes ...
[info] [info] Done compiling.
[info] [info] running B
[info] 1
[info] [success] Total time: 3 s, completed Sep 2, 2020 2:32:47 PM
[info] [info] Compiling 1 Scala source to /private/var/folders/m4/4dsrk6zn74v3zzpk34nj11mh0000gn/T/sbt_e6a419e4/inner-class/target/scala-0.28/classes ...
[info] [info] Done compiling.
[info] [success] Total time: 0 s, completed Sep 2, 2020 2:32:47 PM
[info] [info] Compiling 1 Scala source to /private/var/folders/m4/4dsrk6zn74v3zzpk34nj11mh0000gn/T/sbt_e6a419e4/inner-class/target/scala-0.28/classes ...
[info] [info] Done compiling.
[info] [info] running B
[info] [error] (run-main-1) java.lang.NoSuchMethodError: A$InnerClass.foo()I
[info] [error] java.lang.NoSuchMethodError: A$InnerClass.foo()I
[info] [error] at B$.main(B.scala:4)
[info] [error] at B.main(B.scala)
[info] [error] at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[info] [error] at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
[info] [error] at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[info] [error] at java.lang.reflect.Method.invoke(Method.java:498)
[info] [error] stack trace is suppressed; run 'last Compile / bgRun' for the full output
[info] [error] Nonzero exit code: 1
[info] [error] (Compile / run) Nonzero exit code: 1
[info] [error] Total time: 0 s, completed Sep 2, 2020 2:32:47 PM |
smarter
force-pushed
the
fix-incremental-inner
branch
from
bb2dee8 to
1d29924
Compare
|
Oops, I broke it before pushing it, should be good now. |
|
Ping for review. |
|
when i run |
bishabosha
requested changes
Oct 15, 2020
smarter
force-pushed
the
fix-incremental-inner
branch
from
1d29924 to
fe1781a
Compare
|
seems a lot of PRs have hanging workflows |
|
There's only one Windows CI runner currently, so windows jobs spend a long time in the queue. |
smarter
force-pushed
the
fix-incremental-inner
branch
from
0252e4b to
fe1781a
Compare
... or maybe the windows CI runner is not running currently? looking at https://github.com/lampepfl/dotty/actions I can't find any job where it's running right now /cc @liufengyun |
I'll try to add a new runner on the same machine and see if we get some speedup. Otherwise, we can either (1) run Windows CI only on master merged; or (2) add more physical machines. |
bishabosha
reviewed
Oct 16, 2020
To communicate to sbt a dependency between a class currently being compiled and some other class on the classpath, we use the `binaryDependency` callback which requires passing the "binary class name": for a class A in a package pkg, this is just "pkg.A", but for an inner class B in A, that would be "pkg.A$B", in other words the last component of the binary class name is the name of the corresponding classfile. Before this commit, we computed this name by extracting it from the path of the `associatedFile` but it turns out that `associatedFile` for an inner Scala class returns the classfile of the corresponding top-level class! This happens because the compiler never actually reads inner Scala classfiles since all the information is present in the top-level .class and .tasty files. This means that under separate compilation a dependency to an inner class was not recorded which could lead to undercompilation (see added tests). This commit fixes this by computing binary class names manually: they're just made of the fullName of the enclosing package, followed by ".", followed by the flatName of the current class. This is similar to what is done in the Scala 2 compiler bridge in Zinc.
smarter
force-pushed
the
fix-incremental-inner
branch
from
fe1781a to
0e350d5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To communicate to sbt a dependency between a class currently being
compiled and some other class on the classpath, we use the
binaryDependencycallback which requires passing the "binary classname": for a class A in a package pkg, this is just "pkg.A", but for an
inner class B in A, that would be "pkg.A$B", in other words the last
component of the binary class name is the name of the corresponding
classfile.
Before this commit, we computed this name by extracting it from the path
of the
associatedFilebut it turns out thatassociatedFilefor aninner Scala class returns the classfile of the corresponding top-level
class! This happens because the compiler never actually reads inner
Scala classfiles since all the information is present in the top-level
.class and .tasty files. This means that under separate compilation a
dependency to an inner class was not recorded which could lead to
undercompilation (see added tests).
This commit fixes this by computing binary class names manually: they're
just made of the fullName of the enclosing package, followed by ".",
followed by the flatName of the current class. This is similar to what
is done in the Scala 2 compiler bridge in Zinc.