Bug 130311 - null reference analysis: false positive with conditional && and ||
Summary: null reference analysis: false positive with conditional && and ||
Status: RESOLVED INVALID
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.2   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: JDT-Core-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-03-03 08:52 EST by Markus Keller CLA
Modified: 2006-03-06 04:58 EST (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2006-03-03 08:52:40 EST
I20060301-0800

    boolean bite(Integer aaa, Integer bbb) {
        return (aaa == null && bbb == null) 
                || (aaa.byteValue() == bbb.byteValue());
    }

Second reference to aaa has wrong warning: "The variable aaa may be null". aaa cannot be null, since the right hand side of || would never have been entered then.
Comment 1 Markus Keller CLA 2006-03-03 08:54:39 EST
If I change the && to &, both aaa and bbb get the warning.
Comment 2 Maxime Daniel CLA 2006-03-03 10:28:11 EST
Adding line numbers for clarity:
1    boolean bite(Integer aaa, Integer bbb) {
2        return (aaa == null && bbb == null) 
3                || (aaa.byteValue() == bbb.byteValue());
4    }

Assuming that aaa is null and bbb is not, aaa == null yields true, bbb == null yields false, hence the condition of line 2 is false and we enter the condition of line 3 with aaa null. The warning on aaa is right.
Now, we miss the warning on bbb.



Comment 3 Markus Keller CLA 2006-03-03 11:01:09 EST
<blush> You're right, and with &, both warnings are correct.

BTW: I found the pattern when debugging bug 130317 (I now filed bug 130330 for the bogus null check).
Comment 4 Maxime Daniel CLA 2006-03-03 13:02:25 EST
I discussed this with Philippe on my way home and he pointed out that we are correct not to complain on bbb (in the && case).
If bbb is null, then:
- either aaa is also null and we return from line 2 with true;
- or else aaa is not null and we return from line 2 with false (without evaluating bbb against null, btw).
Hence we're right not to report that bbb may be null on line 3.

With &, this is another story, since aaa non null bbb null is a valid combination that goes to line 3.
Comment 5 Maxime Daniel CLA 2006-03-06 04:58:39 EST
Try harder...
My second case above is wrong.
If aaa is non null, then we go to line 3 without having evaluated bbb against null. This results into bbb not being suspect of being null, hence no warning (that is, we evaluate the expression before returning, instead of returning immediately as I stated above).
As a result, we cannot infer on line 4 that bbb is non null, which shows up in the following test case, in which we have no warning on line 3:

1    boolean bite(Integer aaa, Integer bbb) {
2        return (aaa == null && bbb == null) 
3                || bbb == null;
4    }

(if we could decide that bbb is definitely non null, we should warn so.)

The question that is left open is should we be conservative, and keep the algorithm as it is, or try to infer from the expression on line 3 that the developer does not trust bbb not to be non null?

I keep the conservative approach for now.