Strange Java compiler warning: incorrect "potential null access warning"

Question

(JDK 1.6.0_23, Eclipse 3.7.0 with "Potential null pointer access" warning level at "Warning") Consider the following example code:

Object obj = null;
for (;;) {
    obj = getObject();
    if (obj != null) break;
    Thread.sleep(25);
}
obj.toString();

I'm getting the following warning on the last line: Potential null pointer access: The variable obj may be null at this location. Is there any real way for obj to actually be null or why the compiler thinks so?

does it give the same warning when you remove `.toString()`? — Bastardo, Jul 21 '11 at 06:38
Which compiler? (JDK 1.6.0_23 `javac` I guess.) What compiler switches? — Stephen C, Jul 21 '11 at 06:40
@JohnnyCageWins no, it doesnt; Stephen: Eclipse 3.7.0 with no changes, edited a post. — Alex Abdugafarov, Jul 21 '11 at 06:43
This is also discussed [here](http://pastebin.com/68h3YxVm). — Lion, Apr 18 '12 at 02:02

Thomas Jung · Answer 1 · 2011-07-21T06:37:27.103

8

I'd say the compiler sees this as:

Object obj = null;
[Too dumb to interpret this loop]
obj.toString();

So obj.toString() could be null if you cannot interpret what the loop does.

For example you can trick the compiler by replacing:

void x(int x){
    return;  
    x++;  //error unreachable code
}

with

void x(int x){
    if(true) return;
    x++;  //compiles
}

edited Jul 21 '11 at 06:37

answered Jul 21 '11 at 06:33

Thomas Jung

32,428
9
84
114

Well, compiler frequently understands much more complicated loops, and I dont get, why cant it analyze this one. – Alex Abdugafarov Jul 21 '11 at 06:36
Yes, last one compiles, but you still get a proper `Dead Code` warning. – Alex Abdugafarov Jul 21 '11 at 06:38
1

Interesting side node: The fact that the compiler is too dumb to analyze the `if (true)` is *specified in the JLS*. It's a requirement. Try replacing it with `while (true)` (which is handled differently in the JLS). – Joachim Sauer Jul 21 '11 at 06:42
1

@Frozen Spider - not really a relevant question. The fact is that it can't. But if you really want to know, look at the source code ... and check to see if there are bug reports in the Bugs database for this oddity. – Stephen C Jul 21 '11 at 06:45
@Joachim Sauer - not strictly true. The compiler is allowed to analyse this (and optimize away the dead code) but it is not allowed to treat it as an "unreachable code" **error**. In fact, this case is rather different to the OP's case. – Stephen C Jul 21 '11 at 06:50
@Frozen Spider - correction to my above: since this the an Eclipse compiler artifact check the Eclipse issues tracker. – Stephen C Jul 21 '11 at 06:54
@Stephen C: I should have made that more clear: I was talking only about the errors specified by the JLS: the compiler *most not* treat the `return;` as unreachable code as defined in the JLS. Is *may* be smart enough and post a warning, but the required dead code detection specified in the JLS does *not* consider this to be dead code. – Joachim Sauer Jul 21 '11 at 12:18
@JoachimSauer AFAIK this requirement is in place to help avoid issues with this kind of code: `private boolean DEV = true; [...] if(DEV) log(something);` – zovits Feb 27 '14 at 14:28

score 4 · Answer 2 · answered Jul 21 '11 at 06:45

It seems that compiler cannot analyze the code very well. It actually cannot "run" it and understand that your if statement prevents null access. I played a little bit with the code and found the following equivalent version that does not produce warning:

    Object obj = null;
    do {
        obj = getObject();
        if (obj == null) {
            Thread.sleep(25);
        }
    } while (obj == null);
    obj.toString();

Yes, I know that this version performs 2 null checks on each loop iteration instead of one. So, it is up to you to change your code, add @SuppressWarning to your code or to remove Thread.sleep(25)

score 3 · Accepted Answer · answered Jul 21 '11 at 06:39

The compiler is seeing this as a potential that the object will not be initialized. It is not really able to see that the loop breaking condition is that the object is not null or it will just run forever or until it is no longer null. I only get the warning on the JDK you mentioned. Using a more updated version this warning does not appear.

score 3 · Answer 4 · answered Jul 21 '11 at 07:06

3

This is an improved version of AlexR's code which I think is a nice solution to the problem while also improving readability of the code. My version features a second obj = getObject(); line, but doesn't need the second check, so it should be more performant overall.

Object obj = null;
obj = getObject();
while (obj == null) {
    Thread.sleep(25);
    obj = getObject();
}
obj.toString();

answered Jul 21 '11 at 07:06

Koraktor

41,357
10
69
99

you are right that your version performs only on null check (+1). But I think that both my `if()` into the loop and your call to `getObject()` before the loop just make code less readable. Probably `@SuppressWarning` on original code is better solution in this specific case. – AlexR Jul 21 '11 at 07:15

score 0 · Answer 5 · answered Aug 30 '13 at 07:53

I know it's an old thread, but how about this? I find it more readable than the versions of Koraktor and AlexR.

Object obj = null;
for (obj=getObject(); obj==null; obj=getObject()) {
  Thread.sleep(25);
}
obj.toString();

But in my code, I usually have enabled "Include 'assert' in null analysis" in eclipse compiler settings and add an assert. So I don't have to change control flow and at the same time avoid the warning:

Object obj = null;    
while (true) {
    obj = getObject();
    if (obj != null) break;
    Thread.sleep(25);
}
assert obj!=null;
obj.toString();

Strange Java compiler warning: incorrect "potential null access warning"

5 Answers5

Linked