-
Notifications
You must be signed in to change notification settings - Fork 874
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
[NETBEANS-3725] Preventing a crash in Flow due to unattributed ASTs (cleared nerrors prevents post attr to run in some cases). #7153
Conversation
…cleared nerrors prevents post attr to run in some cases).
👍 tested locally: 21 got me NPEs, this PR made it work (re Find usages and Call Hierarchy) |
i think we should merge this soon since it looks like it is fixing at least part of the problem (see #3725 (comment)). This would close many "NPE in javac" issues and hopefully make it easier to track down the remaining problem. |
merged as tested + approved |
This PR has not fixed class or method renaming, despite claims made on the mailing list. Now Netbeans 22 RC 3 just fails silently, leaving projects in a broken state. There is no difference in the pass/fail before this PR and after as far as I can tell. |
javac processes code in several phases, and one of the phase is "analyze" which looks into method bodies, attaches attributes to the AST nodes and looks for various errors in the method bodies. It consists of two sub-phases, Attr and Flow.
It sometimes may happen that Attr leaves some fields in the AST unfilled when the code is erroneous. In the normal way javac works, this is not problematic, as if there's a compile-time error reported during Attr, Flow does not run. But, when running inside NetBeans, we push javac to Flow regardless of any errors anywhere in the compilation pipeline. To facilitate that, javac will fill various error-like types/symbols on the AST after Attr, if it continues with Flow, and if error was reported.
Up to this point, everything works reasonably.
But, the current code in NetBeans is clearing the number of reported errors, and then javac cannot detect that an error was reported, and fill not fill the AST with the error-like attributes. And this then may lead to a crash in Flow.
I believe the clearing of nerrors was needed to force javac to continue processing even after an error was detected, before the introduction of the current
-Dshould-stop.at=FLOW
approach. But, I don't think it is needed (on most places) currently, and is actually harmful, as described above.So, this patch removes the
nerrors = 0
on all places I could find except inVanillaCompileWorker
. The situation in theVanillaCompileWorker
is different - we only clearnerrors
after we fix (or at least try to) the AST to not contain errors.