Eclipse gives dead code warning for reachable code (variant)

Question

I have following code:

public String myMethod(String keyValue) {
    Map<String, Integer> keyValueToRowIndex = ...
    Integer rowIndex = (keyValue == null) ? 0 : keyValueToRowIndex.get(keyValue);
    if (rowIndex == null)
      return null;
    ...
}

Eclipse gives a "dead code" warning on the return null;. Removing the test for keyValue == null also removes the warning but I don't see how that extra test makes the return statement dead code. Clearly if the map contains no entry for some non-null keyValue, then rowIndex can still be null. Or am I missing something here?

I've seen similar Eclipse issues (here for instance), but this one seems a different and more trivial one.

What is your Java version? there is a difference between Java 7 and 8. https://stackoverflow.com/a/25417505/5168011 — Guy, Oct 15 '18 at 10:45
I had it on Java8, but the warning is there both with 7 and 8. — geert3, Oct 15 '18 at 10:50
Also it throws NPE in both cases (see comment on accepted answer) — geert3, Oct 15 '18 at 10:55

score 3 · Answer 1 · answered Oct 15 '18 at 10:59

The (surprising) short answer: Eclipse is right! This is dead code!

Reason

The important part is the ternary expression in the following line of code:

    Integer rowIndex = (keyValue == null) ? 0 : keyValueToRowIndex.get(keyValue);

The Java language specification (JLS) says about the "Conditional Operator ?", that, if the first expression is of type int, and the second expression is of type Integer, the type of the entire expression will be int.

In your case, the first expression is the constant literal value 0, which is an int. The second expression is the result of the get method, which returns an object of type Integer. So according to the JLS, the entire expression has primitive type int!

This means, if the second expression (the get-call) will be evaluated, the result will be unboxed from Integer to int. This int value will then be auto-boxed again into an Integer to be able to assign it to the left operand, rowIndex.

But what happens, if the map returns a null value? In this case, unboxing from Integer to int is not possible, and a NullPointerExpression will be thrown!

So eclipse is right, as your expression can never return null, rowIndex will never be null either and the then-block of your if-statement will never be executed and hence is dead code!

Solution

The solution is simple: Use an Integer object instead of an primitive int value for your first expression:

Integer rowIndex = (keyValue == null) ? Integer.valueOf(0) : keyValueToRowIndex.get(keyValue);

thanks, yes this is the full answer. I had already deduced that from @Gyro Gearless' answer, and accepted that. +1 for the references. — geert3, Oct 15 '18 at 11:02

score 1 · Accepted Answer · answered Oct 15 '18 at 10:43

1

My guess is that line 3 is interpreted as

Integer rowIndex = Integer.valueOf((keyValue == null) ? 0 : keyValueToRowIndex.get(keyValue).intValue());

(so both arguments to ?: are unified as int) - oddly enogh, Eclipse now shows no warning, even if it is now obvious that rowIndex is never null...

You might also replace the 0 with Integer.valueOf(0) to make the warning disappear.

answered Oct 15 '18 at 10:43

Gyro Gearless

5,131
3
18
14

You're right, the issue is not in Eclipse but in my code. The ternary expression provokes an autobox to `int`, so if `keyValueToRowIndex.get(keyValue)` returns null, a NullPointerException is thrown on line 3. Thanks for pointing this out. Replacing the `0` with `Integer.valueOf(0)` solves the issue. – geert3 Oct 15 '18 at 10:52

Eclipse gives dead code warning for reachable code (variant)

2 Answers2