top | item 22482688

(no title)

leibnitz27 | 6 years ago

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.

discuss

order

vbezhenar|6 years ago

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.