Fix #7494: check type bounds during java joint compilation #9370
Conversation
TheElectronWill
force-pushed
the
dont-trust-java
branch
2 times, most recently
from
7a213be to
b9f9260
Compare
|
There's one failure in the community build:
Which is probably related to the sbt.ExtractDependencies now being run on java sources. I didn't realize we had all this stuff before PostTyper which should not be run on Java units indeed. I think the simplest way to achieve that might be to go back to dropping these units after Frontend, and just have Frontend take care of running the checks on the Java units before it drops them. |
TheElectronWill
force-pushed
the
dont-trust-java
branch
from
67312da to
eaf39cb
Compare
There was a problem hiding this comment.
Looks good! Several approaches were tried in this PR, but I don't think it's useful to keep that in the history, so I suggest squashing this down to two commits: one for the indentation-related changes and one for everything else (and feel free to describe in the commit message why the current approach was chosen in the end)
TheElectronWill
force-pushed
the
dont-trust-java
branch
from
eaf39cb to
b528fb5
Compare
|
I agree, it's clearer that way. I've put some explanations in the commit message. |
Checking every Select of a java-defined symbol would be too costly and still not perfect. The implemented compromise is to check bounds during the compilation of .java source files. To do this, a new JavaChecks file is added and called by FrontEnd. We can't simply discard java compilation units after PostTyper (instead of after Typer), because sbt.ExtractDependencies (and maybe others) runs before PostTyper and cannot process java sources.
TheElectronWill
force-pushed
the
dont-trust-java
branch
from
b528fb5 to
2bc92db
Compare
Something goes wrong when we try to do mutually recursive F-bounds checking in Java units. I am not sure what exactly. But in any case, Scala should not try to do Java's typechecking. So we now check applications in Java units only if they refer to Scala classes. I added a test for scala#7494, which was originally addressed by scala#9370, the PR which introduced the Java bounds checking. Adding the test ensures that the new restrictions still glag the original error. Fixes scala#17763
Something goes wrong when we try to do mutually recursive F-bounds checking in Java units. I am not sure what exactly. But in any case, Scala should not try to do Java's typechecking. So we now check applications in Java units only if they refer to Scala classes. I added a test for #7494, which was originally addressed by #9370, the PR which introduced the Java bounds checking. Adding the test ensures that the new restrictions still flag the original error. Fixes #17763
As @smarter pointed out on gitter, checking every
Selectof a java-defined symbol would probably be very costly (and still not provide a bullet-proof solution). With this PR, java compilation units are discarded just before Pickler and thus go through additional checks.