Bugzilla – Full Text Bug Listing
| Summary: | [compiler][null] throw-catch analysis for null flow could be more precise | ||
|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Stephan Herrmann <stephan.herrmann> |
| Component: | Core | Assignee: | JDT-Core-Inbox <jdt-core-inbox> |
| Status: | CLOSED WONTFIX | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | amj87.iitr, daniel_megert |
| Version: | 3.8 | Keywords: | helpwanted |
| Target Milestone: | --- | ||
| Hardware: | Other | ||
| OS: | Linux | ||
| See Also: | https://bugs.eclipse.org/bugs/show_bug.cgi?id=453483 | ||
| Whiteboard: | |||
For a checked exception this should work fine, but seems like the loop introduces some complexity. See that this already works
void fooNull(LineNumberReader lineReader) throws IOException, MyException {
Object o = null;
try {
o = new Object();
callSome(); // only this can throw MyException
} catch (MyException e) {
o.toString(); // bogus Pot. NPE warning
}
}
Here's another fishy example:
void experiment1(boolean b) {
Object o = null;
try {
System.out.println("read");
} finally {
o = new Object();
}
if (o == null) // (A)
System.out.println("null");
try {
if (b) throw new IOException();
try {
} finally {
if (b) throw new IOException();
}
} catch (IOException re) {
}
if (o == null) // (B)
System.out.println("null");
}
At (A) we know that o cannot be null so we warn and next line is flagged dead.
At (B) we no longer have this knowledge, despite that fact that o hasn't been touched between the two locations.
If I change IOException to RuntimeException I see the same problems at (A) and (B).
Currently, this forces me to ignore POTENTIALLY_NONNULL during resource analysis, because ResourceLeakTest#test056n() would give a false positive. Inversely, test064() must currently remain silent, although a pot.leak warning would be correct.
Code-ref: In TryStatement#analyseCode() we have two blocks checking for isUncheckedCatchBlock(i). The difference between these two blocks doesn't seem 100% kosher.
Related to the above?
(In reply to comment #2) > Inversely, test064() must currently remain silent, although a pot.leak warning > would be correct. Delete that part. This test (which I haven't even published yet..) incorrectly produced POTENTIALLY_NONNULL, which meanwhile I could fix to producing POTENTIALLY_NULL and all is well on that side. The main issue of comment 2 remains, though. x-ref: in bug 345305 I started experiments which *might* be related to this issue. See bug 453483 comment 5, to be handled here. Another candidate: bug 195638 comment 10. Too much on my plate for 4.6. Bulk deferral to 4.7 Ran out of time for 4.7. Bulk move to 4.8. bulk move out of 4.8 |
void fooNull(LineNumberReader lineReader) throws IOException, MyException { Object o = null; try { while (lineReader.readLine() != null) { o = new Object(); callSome(); // only this can throw MyException } } catch (MyException e) { o.toString(); // bogus Pot. NPE warning } } private void callSome() throws MyException {} Here we report a pot. NPE although that specific catch block can only be reached with o != null. I'm not sure if the throw-catch analysis has enough information to improve here, but we should have a look.