8349132: javac Analyzers should handle non-deferrable errors

[lahodaj]

Consider this test script:

$ cat JDK8349132.sh 
mkdir -p JDK-8349132
cd JDK-8349132
cat >Utils.java <<EOF
package test;
public class Utils {
    public static void run(Task<Param> uat) {}
}
interface Task<T> {
    public void run(T t) throws Exception;
}
class Param {}
EOF
cat >Test.java <<EOF
package test;
public class Test {
    private static void test() {
        Utils.run(new Task<Param>() {
            @Override
            public void run(Param parameter) throws Exception {
            }
        });
    }
}
EOF

javac -d out Utils.java
rm out/test/Param.class
javac -XDfind=diamond -XDshould-stop.at=FLOW -classpath out Test.java

It fails with:

$ bash JDK8349132.sh 
Test.java:4: error: cannot find symbol
        Utils.run(new Task<Param>() {
                           ^
  symbol:   class Param
  location: class Test
Test.java:6: error: cannot find symbol
            public void run(Param parameter) throws Exception {
                            ^
  symbol: class Param
Test.java:4: error: cannot access Param
        Utils.run(new Task<Param>() {
             ^
  class file for test.Param not found
3 errors
An exception has occurred in the compiler (21.0.5). Please file a bug against the Java compiler via the Java bug reporting page (https://bugreport.java.com) after checking the Bug Database (https://bugs.java.com) for duplicates. Include your program, the following diagnostic, and the parameters passed to the Java compiler in your report. Thank you.
java.lang.AssertionError: Analyzer error when processing: Utils.run(new Task<Param>(){
    
    () {
        super();
    }
    
    @Override
    public void run(Param parameter) throws Exception {
    }
});:java.lang.NullPointerException: Cannot invoke "com.sun.tools.javac.code.Type.getTypeArguments()" because "com.sun.tools.javac.util.List.get(int).type" is null
jdk.compiler/com.sun.tools.javac.comp.Analyzer$DiamondInitializer.process(Analyzer.java:258)
jdk.compiler/com.sun.tools.javac.comp.Analyzer$DiamondInitializer.process(Analyzer.java:228)
jdk.compiler/com.sun.tools.javac.comp.Analyzer.doAnalysis(Analyzer.java:577)
jdk.compiler/com.sun.tools.javac.comp.Analyzer$2.flush(Analyzer.java:547)
jdk.compiler/com.sun.tools.javac.comp.Analyzer.flush(Analyzer.java:591)
jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1425)
jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1393)
jdk.compiler/com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:976)
jdk.compiler/com.sun.tools.javac.main.Main.compile(Main.java:319)
jdk.compiler/com.sun.tools.javac.main.Main.compile(Main.java:178)
jdk.compiler/com.sun.tools.javac.Main.compile(Main.java:64)
jdk.compiler/com.sun.tools.javac.Main.main(Main.java:50)
        at jdk.compiler/com.sun.tools.javac.util.Assert.error(Assert.java:162)
        at jdk.compiler/com.sun.tools.javac.comp.Analyzer.doAnalysis(Analyzer.java:579)
        at jdk.compiler/com.sun.tools.javac.comp.Analyzer$2.flush(Analyzer.java:547)
        at jdk.compiler/com.sun.tools.javac.comp.Analyzer.flush(Analyzer.java:591)
        at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1425)
        at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1393)
        at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:976)
        at jdk.compiler/com.sun.tools.javac.main.Main.compile(Main.java:319)
        at jdk.compiler/com.sun.tools.javac.main.Main.compile(Main.java:178)
        at jdk.compiler/com.sun.tools.javac.Main.compile(Main.java:64)
        at jdk.compiler/com.sun.tools.javac.Main.main(Main.java:50)
printing javac parameters to: /tmp/JDK-8349132/javac.20250131_133856.args

The reason for this is as follows: Some errors reported by javac have the "NON_DEFERRABLE" flag, which means the error should not be stored in DeferredDiagnosticHandler, but rather should be reported immediately.

This is particularly the case when e.g. entering the sources before annotation processing - the errors found during this enter run may be resolved by the AP. So, ordinary errors are stored in DeferredDiagnosticHandler and reported later if needed. Non-deferrable errors are not stored, but reported immediately, as those are typically serious errors that cannot be fixed by AP.

javac also has an Analyzer, which looks for code simplifications by speculatively simplifying the code, and re-attributing it to see if it would still compile. It is using DeferredDiagnosticHandler, so that if any errors are found during the speculative attribution, the code simplification is rejected.

But, if a non-deferrable error is reported during the speculative attribution, it is passed through DeferredDiagnosticHandler undetected, and the Analyzer may try to do further evaluations - which may crash the compiler.

The proposal in the PR is to catch all errors while doing this speculative attribution for the Analyzer. As a consequence, the speculative code rewrite is found to be problematic, is rejected, and there's no crash.

Note this will also avoid the third error reported in the above case - that error comes directly from the speculative attribution, and it seems incorrect to report it.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Warning

 ⚠️ Found leading lowercase letter in issue title for 8349132: javac Analyzers should handle non-deferrable errors

Issue

• JDK-8349132: javac Analyzers should handle non-deferrable errors (Bug - P4)

Reviewers

• Aggelos Biboudis (@biboudis - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/23387/head:pull/23387
$ git checkout pull/23387

Update a local copy of the PR:
$ git checkout pull/23387
$ git pull https://git.openjdk.org/jdk.git pull/23387/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 23387

View PR using the GUI difftool:
$ git pr show -t 23387

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/23387.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back jlahoda! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@lahodaj This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8349132: javac Analyzers should handle non-deferrable errors

Reviewed-by: abimpoudis

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 92 new commits pushed to the master branch:

• 9aa6d09: 8326485: Assertion due to Type.addMetadata adding annotations to already-annotated type
• 2cce5ee: 8349142: [JMH] compiler.MergeLoadBench.getCharBV fails
• 305bbda: 8348402: PerfDataManager stalls shutdown for 1ms
• 651ac3c: 6899304: java.awt.Toolkit.getScreenInsets(GraphicsConfiguration) returns incorrect values
• 2d6045a: 8301875: java.util.TimeZone.getSystemTimeZoneID uses C library default file mode
• 13d852a: 8349017: Update ML tests to verify against ACVP 1.1.0.38 version
• 137ad5d: 8348976: MemorySegment::reinretpret should be force inlined
• 7764742: 8348909: [BACKOUT] Implement a better allocator for downcalls
• fcd5ebc: 8292944: Noisy output when running make help the first time
• bba6a8f: 8349143: All make control variables need special propagation
• ... and 82 more: https://git.openjdk.org/jdk/compare/a1fd5f4e88f52125eef4feea91a60641981177c1...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

@lahodaj The following label will be automatically applied to this pull request:

• compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (316b1e2c)

[biboudis]

Looks good to me. And thanks for the great analysis.

I was wondering, the third error is not coming from the DiamondInitializer is seems. It must have been one of the subsequent, right?: LambdaAnalyzer, RedundantTypeArgAnalyzer, RedundantLocalVarTypeAnalyzer, RedundantLocalVarTypeAnalyzerForEach.

[lahodaj]

Looks good to me. And thanks for the great analysis.

I was wondering, the third error is not coming from the DiamondInitializer is seems. It must have been one of the subsequent, right?: LambdaAnalyzer, RedundantTypeArgAnalyzer, RedundantLocalVarTypeAnalyzer, RedundantLocalVarTypeAnalyzerForEach.

The third error comes from the DiamondInitializer (I checked, the rewrites are queued, and run later, but I printed the analyzer that scheduled the rewrite).

[lahodaj]

/integrate

[openjdk]

Going to push as commit 3cf04de.
Since your change was applied there have been 131 commits pushed to the master branch:

• 30f7162: 8349417: Fix NULL usage from JDK-8346433
• 64bd8d2: 8349511: [BACKOUT] Framework for tracing makefile inclusion and parsing
• 5e1cc08: 8349504: Support platform-specific JUnit tests in jpackage
• 89e5e7a: 8349155: The "log" parameter to Lint.logIfEnabled() is not needed
• aad6664: 8333569: jpackage tests must run app launchers with retries on Linux only
• b499c82: 8349383: (fs) FileTreeWalker.next() superfluous null check of visit() return value
• 379c3f9: 8347836: Disabled PopupMenu shows shortcuts on Mac
• 82bc0a7: 8344316: security/auth/callback/TextCallbackHandler/Password.java make runnable with JTReg and add the UI
• 2f2f7cf: 8349084: Update vectors used in several PQC benchmarks
• b9b62a0: 8346792: serviceability/jvmti/vthread/GetThreadState/GetThreadState.java testObjectWaitMillis failed
• ... and 121 more: https://git.openjdk.org/jdk/compare/a1fd5f4e88f52125eef4feea91a60641981177c1...master

Your commit was automatically rebased without conflicts.

[openjdk]

@lahodaj Pushed as commit 3cf04de.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.