Bug 298139

Comment 1 Ayushman Jain 2009-12-23 03:53:40 EST

The problem reported here most probably has the same diagnosis as that reported in bug 190170. The null analysis for binary expressions seems to be insufficient to handle these cases. Both these bugs must be taken care of simultaneously.

Comment 2 Ayushman Jain 2010-01-11 02:37:51 EST

Created attachment 155712 [details]
a possible fix 

Here's a possible approach that fixes the warnings with both bitwise-and n bitwise-OR. However, after further analysis, I found out that it also goes against the JLS specification :

Chapter 16 Pg 528 says:
"Except for the special treatment of the conditional boolean operators &&, ||,
and ? : and of boolean-valued constant expressions, the values of expressions
are not taken into account in the flow analysis.
For example, a Java compiler must produce a compile-time error for the code:
{
int k;
int n = 5;
if (n > 2)
k = 3;
System.out.println(k);// k is not “definitely assigned” before this
}
even though the value of n is known at compile time, and in principle it can be
known at compile time that the assignment to k will always be executed (more
properly, evaluated). A Java compiler must operate according to the rules laid out
in this section. The rules recognize only constant expressions; in this example, the
expression n > 2 is not a constant expression as defined in §15.28. "

Due to this limitation, the patch actually results in making the analysis more intelligent than it should be. The failing test case NegativeTest#test015() demonstrates the resulting problem. Even though it is evident in that test case that the variable b2 will definitely be initialised, the JLS specification actually implies that the compiler should not do such analysis and report an error.

Thus, i believe the current minimal analysis we do for bitwise-AND n bitwise-OR is consistent with the JLS and should be carried forward without any change. Comments invited.

Comment 3 Ayushman Jain 2010-02-01 04:35:29 EST

There are a few more problems i found in my analysis:

In the analysis of || and && , we take care of special cases such as FALSE | anything, TRUE & anything, etc. We cant take care of such special cases in the analysis of bitwise OR and AND, because of the JLS specification given in the above comment. Ignoring these nullifies most of the improvement we can make in analysing these. Also, unlike for || and &&, we cant make the opportunistic use of ConditionalFlowInfo for | and & since short circuiting cant be done.

Comment 4 Lance Walton 2010-02-01 16:01:08 EST

Hi.

Thanks for the comments. I must admit, I'm having trouble understanding the reasoning though:

    public void test(final Object foo, final Object bar) {
        if (foo == null | bar == null) {
            return;
        }
        System.out.println(bar.toString());
    }

We can deduce that there is no way that the flow of control can get to the System.out if 'bar' is null. Therefore, raising a warning that suggests that bar might be null at that point seems wrong. We know that 'bar' is 'definitely assigned' in the above fragment, so I'm not sure that that point in the JLS applies.

Anyway, I'm not criticising - you guys do a great job. I'd just like not to have valid warnings obscured by 'false' ones.

Regards,

Lance

Comment 5 Ayushman Jain 2010-02-02 08:53:29 EST

(In reply to comment #4)

> Thanks for the comments. I must admit, I'm having trouble understanding the
> reasoning though:
> 
>     public void test(final Object foo, final Object bar) {
>         if (foo == null | bar == null) {
>             return;
>         }
>         System.out.println(bar.toString());
>     }
> 
> We can deduce that there is no way that the flow of control can get to the
> System.out if 'bar' is null. Therefore, raising a warning that suggests that
> bar might be null at that point seems wrong. We know that 'bar' is 'definitely
> assigned' in the above fragment, so I'm not sure that that point in the JLS
> applies.

I understand that it seems quite intuitive to anyone here that that bar can never be null in bar.toString(), and in my analysis for this bug, I'd easily fixed this issue. This is where the problem occured.

To explain it further, we currently analyse the expression to the left of '|' and then we superimpose the obtained info with analysis from the expression to the right. This results in some information loss (can't go into complete mechanism here) which results in the false warning. Ideally, we should analyse the | expression by analysing the left and right expressions individually and then composing the info as follows:
-> The true part is composed of the info when either left, or right, or both expressions are true.
-> The false part is composed of the Info when both are false.

To do the above we use "merge"

Now consider the case:

boolean foo = false, bar;
if (foo == false | (foo||(bar = false)) {}
else foo = !bar;

Now according to javac, we should have a "bar may not be initialised" error on "else foo = bar" (although its again very intuitive to the human eye that bar will always be initialized here). However, in case we improve analysis for '|', where we merge the info from the left and right expressions to obtain the perfectly composed info, we also include the info of bar getting initialised.Hence we dont get the may not be initialized error! This is the thing that violates the JLS spec. 

So basically your case can be fixed, but at the expense of risking violating some JCK test, which we cannot afford. 

Thanks! :)

Comment 6 Kenney 2011-12-02 16:25:04 EST

This is not a bug.

Normally the potential null pointer warning is not active, unless a null check occurs (as per observation). After that, the variable is considered potentially null.

The code (foo == null | bar == null) evaluates to (boolean | boolean),
since | is a bitwise or, not a logical or.

At this point, the return statement can only be known to be executed
if either foo or bar (or both) are null. 

After the return statement, all that is known is that either foo or bar is not
null.

Hence the warning.

Comment 7 Eclipse Genie 2019-11-11 20:14:04 EST

This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.