issues with by-file compilation policy #781
Comments
I think so, assuming you're using lombok? |
|
No, I’m not. Do you need some more information?
|
|
Do you have a self-contained example that reproduces the crash? |
|
I will have to try. I’ll write tomorrow.
|
|
@cushon - OK, it took me an hour or so, it was horribly complicated to extract a reproduction, so you owe me a beer! :) Or Google. :D Here goes: package a;
import java.util.function.Function;
class Bar {
private final Function<String, String> args;
public Bar(Function<String, String> args) {
this.args = args;
}
}
public abstract class AbstractFoo {
protected abstract String getFoo();
private String getCommandArguments(String parameters) {
return null;
}
public AbstractFoo() {
new Bar(this::getCommandArguments);
}
}package a.b;
import a.AbstractFoo;
class Baz extends AbstractFoo {
@Override
protected String getFoo() {
return "foo";
}
}A few things to note - the second file MUST be in a different package and also in a child package. Also, That's a pretty insane bug. :) |
|
@boris-petrov thanks! 🍺🍺🍺 Verified with The AST node it's crashing on is: which means that Error Prone is seeing a compilation unit after javac has already lowered the method reference, which isn't supposed to happen. This is a javac bug with the default compilation policy that can be worked around by setting We should make sure the build system integrations are doing that, and also have Error Prone check the policy when it runs and complain if it's one of the broken ones. We already do some of that here, but at the time that was added we though the |
|
@cushon are there any downsides to passing |
|
It uses slightly more memory, but not an amount that I suspect matters. If you want to verify that for your usage, I'd be interested in what you find. Internally we're using a fix to |
|
We'll give it a try and if we see any serious memory regression we'll report back. +1 to including your internal fix in |
|
I'm not sure I followed the conversation. What do you suggest we do to fix this issue? I am hitting it on Java version |
|
Passing the javac flag I think we should either make Error Prone always pass that flag, or check that the flag is enabled before running, or make a fix to the javac we redistribute. |
The default policy may results in compilation units being lowered before Error Prone sees them. (context: b/36444786, b/36098770, b/27686620) RELNOTES: N/A ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=172897895
The default policy may results in compilation units being lowered before Error Prone sees them. (context: b/36444786, b/36098770, b/27686620) RELNOTES: N/A ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=172897895
cushon
changed the title
When -XDcompilePolicy=BY_FILE is enabled, javac groups symbols by compilation unit and moves them through the phases of compilation together. A symbol cannot be desugared until its ancestors have been desugared, so javac also tracks supertypes and ensures they get desugared first. The second ordering constraint was overriding the first, and causing symbols to be desugared separately from their compilation unit group. See: * google/error-prone#781 * https://bugs.openjdk.java.net/browse/JDK-8155674
|
This is fixed by fcbbd71 (specifically google/error-prone-javac@b5e0946). |
RELNOTES: N/A ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=182878699
When -XDcompilePolicy=BY_FILE is enabled, javac groups symbols by compilation unit and moves them through the phases of compilation together. A symbol cannot be desugared until its ancestors have been desugared, so javac also tracks supertypes and ensures they get desugared first. The second ordering constraint was overriding the first, and causing symbols to be desugared separately from their compilation unit group. See: * google/error-prone#781 * https://bugs.openjdk.java.net/browse/JDK-8155674
Please tell me if you need any more information.
P.S. This is probably a duplicate of #780 but I'll leave you to decide if it is.
The text was updated successfully, but these errors were encountered: