Hacker News new | ask | show | jobs
by vbezhenar 2291 days ago
Looks like a bug. But anyway should be easily detectable by a static analysis and reported as a warning, so not a big deal in practice, even if working as intended.
1 comments

Nope - the if condition is not considered in flow analysis. Read the end:

https://docs.oracle.com/javase/specs/jls/se8/html/jls-14.htm...

"14.21. Unreachable Statements It is a compile-time error if a statement cannot be executed because it is unreachable.

This section is devoted to a precise explanation of the word "reachable." 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."

I still don't understand why reachable or unreachable changes the binding from names to local variable/field. The process of identifier resolution should happen before reachability analysis.

Is there some bug or mail list thread with reaction from Java developers?

I haven't raised mine, as I consider it to be a refinement of the bug noted in https://twitter.com/tagir_valeev/status/1210431331332689920 (Don't know if java devs have responsed to that.)

(again, reachability analysis of unrelated code changes semantics.)

The problem is that this IS defined behaviour - the scope of the instanceof-assigned variable is dependent on whether or not the taken if-statement is provably exiting.

This is intended to allow

  {
    if (!(obj instanceof String s)) return;

    // s exists now.
  }
But it's not been thought through.
IMO they should treat it exactly like they treat uninitialized variables.

    void test1(boolean b) {
        String s;
        if (b) {
            return;
        } else {
            s = "test";
        }
        System.out.println(s);
    }
This code compiles.

    void test2(boolean b) {
        String s;
        if (b) {
            if ("a".equals("a")) {
                return;
            }
        } else {
            s = "test";
        }
        System.out.println(s);
    }
This code does not compile. But it does not mean that println will try to resolve s to something else. I think that they should have gone to a similar route, where declared variable will be available for entire lexical block where `if` was used, but initialized only inside matched branch. Usage in other code would error with "variable might not have initialized" consistently how it works now.

Of course that would require to shadow previous declaration for consecutive `if`-s. But it would be much more obvious and understandable. Actually the whole construction would be just a syntax sugar almost expressible with current Java constructions:

        /*
        if (o instanceof String s) {
            System.out.println(s.length());
        }
        //System.out.println(s.length()); // variable might not have initialized
        if (o instanceof Number s) {
            System.out.println(s.intValue());
        }
         */
        String s;
        if (o instanceof String) {
            s = (String) o;
            System.out.println(s.length());
        }
        //System.out.println(s.length()); // variable might not have initialized
        Number s$; // no variable shadowing in Java now, but it could work
        if (o instanceof Number) {
            s$ = (Number) o;
            System.out.println(s$.intValue());
        }
        //System.out.println(s$.intValue()); // variable might not have initialized
and would be directly expressible if Java would allow variable name shadowing which is a good thing as proven by Go and Rust (although that would be incompatible change for old code, but allowing variable shadowing for patterns would not be incompatible change, because old code does not have pattern variables).

Of course I did not think about this problem for too long and probably missed something important, so that's just my 2 cents. I guess, developers took that path for a reason.

Basically they want to following code to work:

    String s;
    void test(Object o) {
        if (o instanceof String s) {
            System.out.println(s); // local variable o
        } else {
            System.out.println(s); // this.s
        }
    }
and I'd argue that this code should not compile! It's bad code. If developer wants to use `this.s` he should explicitly write that.
It's a discussion of a simple use-case where variable is only available inside positive if match (actually pretty logical behaviour). It does not touch unreachable code modifying lexical analysis (what really causes confusion).