0

Why do I get no dead code warning for the initialisation of someVal here?

public class DeadCode {

    private int someVal = 0;

    public DeadCode(int someVal) {
        this.someVal = someVal;
    }

    public int getSomeVal() {
        return someVal;
    }

    public void setSomeVal(int someVal) {
        this.someVal = someVal;
    }

}

The Java compiler is supposed to pick up on dead code and issue a warning; but this is dead twice over, and passes by without a hitch.

It's dead twice over because

  1. Java automatically initialises instance fields to 0 or equivalent;
  2. the value of someVal can't be read without being written to.

I realise that the compiler can elide the assignment if it wants to, but that's true (by definition) of all dead code.

If there is a distinction to be made between dead code and code that has no effect, then I would expect

The assignment to variable someVal has no effect.

which is what I would get if I wrote

someVal = someVal;

in my code. But I don't get that either.

In any case, Wikipedia sees dead code elimination as removal of code that has no effect on program results; and this is certainly a case of that.

chiastic-security
  • 20,430
  • 4
  • 39
  • 67
  • 6
    Redundant code and dead code are not the same. – azurefrog Nov 11 '14 at 20:27
  • 1
    I cannot see the dead code. This situation occurs when compiler considers that some of your code will never be executed. Your example is just two different ways how you can initialize instance parameter and change it through a method on-the-fly – energizer Nov 11 '14 at 20:30
  • @energizer No, that's unreachable code. Dead code is code that has no effect, which is what this is. – chiastic-security Nov 11 '14 at 20:33
  • @azurefrog I've updated the question to cover this. But Wikipedia thinks dead code elimination is about [removing code that will not affect output](http://en.wikipedia.org/wiki/Dead_code_elimination), which is exactly what this is. – chiastic-security Nov 11 '14 at 20:35
  • @chiastic-security In eclipse at least, unreachable code is code which *cannot* be executed (e.g. code after a **return** statement), and dead code is code which *will not* be executed (e.g. `if (a) { if (!a) { //this is dead } }`). Code that has no effect, but will be executed is neither unreachable nor dead. – azurefrog Nov 11 '14 at 20:37
  • There is another approach. I think that the dead code can be represented like this: assume that you have some computation: result = a + b; return a+b; - so the 'result' will never be used at all, so compiler will mark it as "unused" or "dead code". Actually concept of 'dead code' is misleading in nowadays because every programmer can interpret it like he understands it. – energizer Nov 11 '14 at 20:43
  • @azurefrog I don't get a warning with your example there either. – chiastic-security Nov 11 '14 at 20:43
  • Either the compiler checks for this case for each field with an initialization expression or the JVM gets rid of it, which I believe it already does. – Sotirios Delimanolis Nov 11 '14 at 20:44

2 Answers2

3

I think the real answer is that many people would find a warning for that code to be pretty annoying.

As a technicality, though, the initializer does not constitute dead code because its effect can, in principle, be observed in an improperly synchronized multithreaded program.

John Bollinger
  • 160,171
  • 8
  • 81
  • 157
  • Can you show how that might happen? I don't think it can. The value isn't accessible by any thread until the constructor and the initializers have finished. There's no way of getting at an uninitialised value in Java, even if you mess about with threads. – chiastic-security Nov 11 '14 at 20:46
  • You are mistaken, @chiastic-security. The field's default value and the value after its initializer runs but before the constructor runs can both, in principle, be seen by other threads before the constructor completes. There's even a name for this synchronization issue: "improper publication". You can find plenty of discussion under that name, including [here](http://stackoverflow.com/questions/16107683/improper-publication-of-java-object-reference). – John Bollinger Nov 11 '14 at 20:52
  • OK. But the default value and the value after initialisation are the same... so how could another thread's behaviour be affected by the initialisation? – chiastic-security Nov 11 '14 at 20:54
  • 1
    In an improper publication scenario, not only could the value be *seen* prior to completion of the constructor, it could also be *modified* via its mutator prior to completion of the constructor. And that mutation could, in principle, be seen too. And then be overwritten by the initializer. – John Bollinger Nov 11 '14 at 20:56
  • That's a good point. I could get rid of the mutator, though, and that problem would then not arise. – chiastic-security Nov 11 '14 at 20:59
  • Not so. Fields, even private ones, can in general be reflectively modified. Security policy can affect that, but the compiler cannot make assumptions about runtime security policy. – John Bollinger Nov 11 '14 at 21:01
  • I think the problem there is that this would make *all* warnings about `x = x` assignments wrong. – chiastic-security Nov 11 '14 at 21:10
  • Yes and no. Yes, the same argument applies to any field initializer, and in fact you observed that initializers you thought constituted dead code did not elicit a warning. No, assignments to local variables of methods and constructors would not be affected because they are not shared between threads. – John Bollinger Nov 11 '14 at 21:19
0

Integer field is a simplistic use case. What if you had something like this, notice the parameterless constructor:

public class DeadCode {

    private List<Integer> someVal = new ArrayList<>();

    public DeadCode() {
    }

    public DeadCode(Iterable<Integer> someVal) {
        this.someVal.addAll(someVal);
    }

    public Iterable<Integer> getSomeVal() {
        return someVal;
    }

    public void addSomeVal(int someVal) {
        this.someVal.add(someVal);
    }
}

You have to create a list, yes you could do it in the constructor Now the compiler needs to be a lot more intelligent to be able to determine in which cases it wasteful to initialize field. I guess they opted for consistent and straightforward behavior.

LeffeBrune
  • 3,441
  • 1
  • 23
  • 36
  • this.someVal.addAll(someVal); --> could be this.someVal = someVal; – StackFlowed Nov 11 '14 at 20:37
  • 1
    This isn't dead at all. If I call the no-arg constructor, and then `addSomeVal(0)`, the field initialisation will avoid a `NullPointerException`. – chiastic-security Nov 11 '14 at 20:41
  • I'm trying to make a point that I'd rather have a compiler that handles all these cases consistently, instead of forcing me to remove a field initialization when it is not necessary. – LeffeBrune Nov 11 '14 at 20:44