views:

1245

answers:

6

Does anyone know why:

public void foo()
{
    System.out.println("Hello");
    return;
    System.out.println("World!");
}

Would be reported as an "unreachable error" under Eclipse, but

public void foo()
{
    System.out.println("Hello");
    if(true) return;
    System.out.println("World!");
}

Only triggers a "Dead code" warning?

The only explanation I can think of is that the Java compiler only flags the first, and that some extra analysis in Eclipse figures out the second. However, if that is the case, why can't the Java compiler figure out this case at compile time?

Wouldn't the Java compiler figure out at compile time that the if(true) has no effect, thus yielding bytecode that is essentially identical? At what point is the reachable code analysis applied?

I guess a more general way to think of this question is: "when is the reachable code analysis applied"? In the transformation of the second Java code fragment to the final bytecode, I am sure that at some point the "if(true)" runtime equivalent is removed, and the representations of the two programs become identical. Wouldn't the Java compiler then apply its reachable code analysis again?

+11  A: 

The first does not compile (you got an error), the second compiles (you just got a warning). That's the difference.

As to why Eclipse detects dead code, well, that's just the convenience of an integrated development tool with a built-in compiler which can be finetuned more as opposed to JDK to detect this kind of code.

Update: the JDK actually eliminates dead code.

public class Test {
    public void foo() {
        System.out.println("foo");
        if(true)return;
        System.out.println("foo");
    }
    public void bar() {
        System.out.println("bar");
        if(false)return;
        System.out.println("bar");
    }
}

javap -c says:

public class Test extends java.lang.Object{
public Test();
  Code:
   0:   aload_0
   1:   invokespecial   #1; //Method java/lang/Object."":()V
   4:   return

public void foo();
  Code:
   0:   getstatic       #2; //Field java/lang/System.out:Ljava/io/PrintStream;
   3:   ldc             #3; //String foo
   5:   invokevirtual   #4; //Method java/io/PrintStream.println:(Ljava/lang/StrV
   8:   return

public void bar();
  Code:
   0:   getstatic       #2; //Field java/lang/System.out:Ljava/io/PrintStream;
   3:   ldc             #5; //String bar
   5:   invokevirtual   #4; //Method java/io/PrintStream.println:(Ljava/lang/String;)V
   8:   getstatic       #2; //Field java/lang/System.out:Ljava/io/PrintStream;
   11:  ldc             #5; //String bar
   13:  invokevirtual   #4; //Method java/io/PrintStream.println:(Ljava/lang/String;)V
   16:  return

}

As to why it (Sun) doesn't give a warning about that, I have no idea :) At least the JDK compiler has actually DCE (Dead Code Elimination) builtin.

BalusC
But why wouldn't the Java compiler eliminate the if(true) at compile time, and then identify the error when it analyzes reachable code?
Uri
Hey Uri, you can replace "if(true)" with "boolean x=true; if(x)". It's not the compiler's job to eliminate it, because you can, for example, put a breakpoint and change the value of x during execution
Yoni
Hopefully my answer (edited a bit since you first saw it) will make that a little clearer.
Carl Smotricz
Also, if the compiler eliminates stuff from the bytecode, you can no longer debug properly because the bytecode won't match your source code
Yoni
+1 The java compiler does not generate bytecode for blocks that do not get executed. Code is still generated if an expression evaluates to false at runtime. The only time bytecode is not produced is when the expression evaluates to false at compile time. At least that's my understanding of how it works.
ChadNC
@Yoni: That's why I specifically put if(true), so that it would be "atomic" and thus (I assumed) eliminated by the compiler.
Uri
I would vote you up, but the first sentence is only "half-right". A Java compiler is *not allowed* to compile the first fragment, and Eclipse can decide not to allow the second, or even to ignore it completely. It is more of a lint style check, the first is mandated by the language. See my answer for more details.
Paul Wagland
That's just a configuration matter. You **can** get second to compile and run. You can't do that with first.
BalusC
@BalusC: I agree, but I think that it is important to point out *why* you can't just ignore the error on the first. It is a language mandated check, the second is just something that the Eclipse developers thought would be useful.
Paul Wagland
@Paul: just because it contradicts the JLS. Read: it's **syntactically invalid**.
BalusC
@BalusC: Fine, put that in your answer then ;-) My point is that the difference is more than just the default compiler settings. There is a *reason* why the first cannot be compiled, and it is not *simply* that the compiler has decided to generate an error, it is that the compiler is *required* by the JLS to generate the error.
Paul Wagland
@Paul: I assumed for obvious that any compilation error is caused by a syntax error :)
BalusC
@BalusC: I am not sure that is so true... For example the default Eclipse settings will Error when you have an import restriction from another project. This is totally Eclipse specific, and not mandated by the language, however I will concede that for the most part, with the default compiler settings, it is true.
Paul Wagland
@Paul: Eclipse is not a spec, it's just a tool.
BalusC
@BalusC: I know, I never tried to imply that Eclipse was a spec. However the project forbidden imports setting is specific to the Eclipse projects. Anyway, it is obvious to me by now, that you are not seeing the difference that I am expressing, so I'll just leave it at that, if anyone wants to know *why* Eclipse doesn't error on the second, but does on the first, I hope that they read my answer, since you are not explaining why, you just say "it is so, because it is so. And that is the difference!"
Paul Wagland
+1  A: 

The if (true) is a little more subtle than "unreachable"; because that hard-coded return will always make the following code unreachable, but changing the condition in the if could make the following statement reachable.

Having a conditional there means that there's a chance condition could change. There are cases where something more complicated than a true is in the parentheses, and it's not obvious to the human reader that the following code is "deadened," but the compiler notices, so it's able to warn you about it.

Eclipse is mentioned here, and it makes things seem a little more complicated to the user; but actually underneath Eclipse is just a (very sophisticated) Java compiler that happens to feature a lot of switches for warnings etc. that Eclipse can switch on and off. In other words, you don't get quite the breadth of different warnings/errors from a straight javac compile, nor do you have convenient means to turn all of them on or off. But it's the same deal, just with more bells and whistles.

Carl Smotricz
I'm not sure I understand the subtlety here. How can the condition change? Since I use the literal, there's no way to change it at runtime.
Uri
Yoni's example under BalusC's answer is a nice practical example of what I was trying to say in my 2nd paragraph.
Carl Smotricz
A: 

The difference is in the semantics between run-time and compile-time. In your second example, the code compiles to an if-else branch in the bytecode, and eclipse is simply smart enough to tell you that the else portion will never be reached in runtime. Eclipse only warns you, because it is still legal code.

In your first example, it is an error because the code is illegal by the definition of java. The compiler doesn't allow you to create byte code with unreachable statements.

Yoni
+5  A: 

Unreachable code is an error according to the Java Language Spec.

To quote from the JLS:

The idea is that there must be some possible execution path from the beginning of the constructor, method, instance initializer or static initializer that contains the statement to the statement itself. The analysis takes into account the structure of statements. Except for the special treatment of while, do, and for statements whose condition expression has the constant value true, the values of expressions are not taken into account in the flow analysis.

What that means, is that the if block is not taken into account, since if you go through one of the paths of the if statement, you could reach final print statement. If you changed your code to be:

public void foo() {
    System.out.println("Hello");
    if (true)
        return;
    else
        return;
    System.out.println("World!");
}

then suddenly it wouldn't compile anymore, since there is no path through the if statement that would allow the last line to be reached.

That is, a Java compliant compiler is not allowed to compile your first code fragment. To further quote the JLS:

As an example, the following statement results in a compile-time error:

while (false) { x=3; }

because the statement x=3; is not reachable; but the superficially similar case:

if (false) { x=3; }

does not result in a compile-time error. An optimizing compiler may realize that the statement x=3; will never be executed and may choose to omit the code for that statement from the generated class file, but the statement x=3; is not regarded as "unreachable" in the technical sense specified here.

The second warning that Eclipse gives, about dead code, is a warning generated by the compiler, that is not "unreachable", according to the JLS, but in practice is. This is an additional lint style check that Eclipse provides. This is entirely optional, and, by using the Eclipse configuration, can be disabled, or turned into a compiler error instead of a warning.

This second block is a "code smell", if (false) blocks are normally put in to disable code for debugging purposes, having it left behind is typically accidental, and hence the warning.

In fact, Eclipse does even more advanced tests to determine the possible values for an if statement to determine whether or not it is possible to take both paths. For example, Eclipse would also complain about dead code in the following method:

public void foo() {
    System.out.println("Hello");
    boolean bool = Random.nextBoolean();
    if (bool)
        return;
    if (bool || Random.nextBoolean())
      System.out.println("World!");
}

It will generate an unreachable code for the second if statement, since it can reason that bool must only be false at this point in the code. In such a short code fragment it is obvious that the two if statements are testing the same thing, however if there are 10-15 code lines in the middle it might not be so obvious anymore.

So in summary, the difference between the two: one is forbidden by the JLS, and one is not, but is detected by Eclipse as a service to the programmer.

Paul Wagland
Interesting that they didn't include 'if' in the list of control flow statements with "special treatment". I wonder why?
meriton
Oh, I get it: the special treatment is to detect obvious infinite loops. 'If' is not a looping statement. (I am a little slow today ...)
meriton
@meriton: I added the appropriate section from the JLS to show what they were trying to resolve.
Paul Wagland
A: 

I think one way to some it up is that unreachable code is most likely a mistake, and the JLS tries to protect you from such mistakes.

Allowing if (true) return; is a good way to work around the JLS limitation if you actually want to do this on purpose. If the JLS stopped this, it would be getting in the way. In addition, it should also stop:

 public static boolean DEBUG = true; //In some global class somewhere else


 ...

 if (DEBUG) return; //in a completely unrelated class.

 ...

Because the DEBUG constant is completely in-lined, and functionally equivalent to just typing a true in that if condition. From a JLS perspective those two cases are very similar.

Yishai
A: 

This is to allow a kind of conditionally compilation.
It is not an error with if, but the compiler will flag an error for while, do-while and for.
This is OK:

if (true) return;    // or false
System.out.println("doing something");

This are errors

while (true) {
}
System.out.println("unreachable");

while (false) {
    System.out.println("unreachable");
}

do {
    System.out.println("unreachable");
} while (true);

for(;;) {
}
System.out.println("unreachable");

It is explained at the end of JLS 14.21: Unreachable Statements:

The rationale for this differing treatment is to allow programmers to define "flag variables" such as:

 static final boolean DEBUG = false;

and then write code such as:

   if (DEBUG) { x=3; }

The idea is that it should be possible to change the value of DEBUG from false to true or from true to false and then compile the code correctly with no other changes to the program text.

Carlos Heuberger