27

The following snippet does not compile on javac, version 17 (Temurin)

class Instanceof {
    static void doesNotWork(Object o) {
        if (o == null) {
            throw new Error();
        } else if (!(o instanceof String s)) {
            throw new Error();
        }   
        System.out.println(s); // error here
    }
}

It generates this error: cannot find symbol

cannot find symbol
symbol:   variable s
location: class Instanceof

However, the following (in my opinion) equivalent variations work: With an explicit else block:

static void doesWork(Object o) {
    if (o == null) {
        throw new Error();
    } else if (!(o instanceof String s)) {
        throw new Error();
    } else {
        System.out.println(s);
    }
}

Or without an else:

static void doesWork(Object o) {
    if (o == null) {
        throw new Error();
    }
    if (!(o instanceof String s)) {
        throw new Error();
    }
    System.out.println(s);
}

Or with a single if:

static void doesWork(Object o) {
    if (o == null || !(o instanceof String s)) {
        throw new Error();
    }
    System.out.println(s);
}

Is this a bug in javac? If yes, should I report this, but where exactly?

Johannes Kuhn
  • 14,778
  • 4
  • 49
  • 73
Fabian Kürten
  • 331
  • 2
  • 6
  • 4
    @RealSkeptic See https://docs.oracle.com/en/java/javase/17/language/pattern-matching.html – Pshemo May 31 '22 at 10:37
  • 3
    The relevant section is [§6.3.1](https://docs.oracle.com/javase/specs/jls/se18/html/jls-6.html#jls-6.3.1) - the error indicates that `javac` thinks variable `s` is not in scope here, given that the error is "s? What s?". It's like a `javac` bug, but I haven't yet found the exact chapter and verse where `javac` breaks spec here. – rzwitserloot May 31 '22 at 10:41
  • 1
    To my untrained eye, it does seem to be an issue. I can't see any logical reason why the examples you've showed would work, but that one wouldn't. You can report a bug here: https://bugreport.java.com/bugreport/ – Michael May 31 '22 at 10:41
  • 6
    I *suspect* this is a difference in the reachability of if-then-else vs if-then, depending on the body of the clauses. – Jon Skeet May 31 '22 at 10:41
  • 1
    Looks scope related, like `s` being available in the `if` scope only… Would be weird if that was the reason… Could be a bug or intentional and your use case is not covered (since you want to print `s` in the outer scope while it is declared in an inner scope). The `else` extends the scope, which explains why that option does work. – deHaar May 31 '22 at 10:42
  • 1
    https://openjdk.java.net/jeps/394 includes an example that is very similar so I think it is a bug. Please report it (link in other comment above)! – ewramner May 31 '22 at 10:45
  • Going by the example (under _"The flow scoping analysis for pattern variables is sensitive to the notion of whether a statement can complete normally."_) seems to suggest both should be equivalent, so I guess it's a bug, but maybe the null check is throwing this flow analysis off here. – Mark Rotteveel May 31 '22 at 10:46
  • 1
    I would have expected that `s` has a possible scope inside the if's `( )` and inside the `{ }`; comparable to _try-with-resources_. Anything else is bad style imho. – Joop Eggen May 31 '22 at 10:47
  • @JoopEggen Kind of agree, but you have to remember that `s` is really just `o` with type-narrowing, and `o` is already in-scope. The point of pattern matching is to make the compiler able to perform some of the same inferences that humans can. A human can infer that `o` is a String after those if-statements, because it would have thrown an exception if it weren't. – Michael May 31 '22 at 10:58
  • @Michael right, but `if (... && !(o instanceof String s) && ...) { ... } ... s` becomes slightly dubious. I hope a more strict ruling will come soon. – Joop Eggen May 31 '22 at 11:08
  • 2
    Works with Eclipse. As a side note, the `null` check is obsolete here… – Holger May 31 '22 at 11:08
  • @JonSkeet not just reachability issue, please check my example in the answers. It seems that this is probably an issue of how the compiler evaluates and transforms the code. – Panagiotis Bougioukos Jun 01 '22 at 09:19
  • 1
    @PanagiotisBougioukos: No, your example seems to actually point to reachability being important, just as I was suggesting... – Jon Skeet Jun 01 '22 at 09:24
  • The second example of [JLS § 6.3.2](https://docs.oracle.com/javase/specs/jls/se18/html/jls-6.html#jls-6.3.2) looks very much like the abovementioned examples. *Looks* to me it's related to how exactly `if`, `else if` and `else` work. Kinda what k314159 says in their answer. – MC Emperor Jun 08 '22 at 09:01

2 Answers2

18

The doesNotWork case is equivalent to this:

static void doesNotWork(Object o) {
    if (o == null) {
        throw new Error();
    } else {
        if (!(o instanceof String s)) {
            throw new Error();
        }
    }
    System.out.println(s); // error here
}

This makes it more obvious that String s is inside a block bounded by curly brackets and is therefore out of scope in the same way that this doesn't work either:

static void doesNotWork(Object o) {
    {
        if (!(o instanceof String s)) {
            throw new Error();
        }
    }
    System.out.println(s); // error here
}

In the case where it does work, with the println inside the else, it's equivalent to this:

if (o == null) {
    throw new Error();
} else {
    if (!(o instanceof String s)) {
        throw new Error();
    } else {
        System.out.println(s);
    }
}

Which shows the println being in scope.

k314159
  • 5,051
  • 10
  • 32
  • Seems like a reasonable guess, but if it were simple a matter of scope then I'd be surprised that Eclipse's compiler wouldn't work the same way. – Michael May 31 '22 at 12:01
  • 3
    Now, the question is, does being equivalent to a block allow treating the code as if being inside a block, even if it’s not? `javac` doesn’t seem to be consistent. E.g. if I change the code to `do if(!(o instanceof String s)) throw new Error(); while(false); System.out.println(s);` it compiles the code (as Eclipse does), despite being equivalent to `do { if(!(o instanceof String s)) throw new Error(); } while(false); System.out.println(s);` which both compilers reject. – Holger May 31 '22 at 12:02
  • I can use the same '_this_ is essentially equivalent to _that_' tactic to say that `int a = 5; System.out.println(a);` is equivalent to `{int a = 5;} {System.out.println(a);}` and thus should not compile: Braces define scope, therefore imagining them here isn't a valid approach. I think this indeed explains what happens, but I'd say this means either the JLS is bugged or javac is. – rzwitserloot May 31 '22 at 12:03
  • 3
    @rzwitserloot note that I intentionally searched for an example with a control flow construct, rather than putting `{ }` around an arbitrary statement, to show that even then, the behavior is not consistent. If the inner statement of a `do … while` loop is only treated as a block if written as a block and, in fact, the entire loop is treated as a single statement when no block is present, this should apply to the `else` clause as well. Like Eclipse does, consistently. – Holger May 31 '22 at 12:16
7

The relevant bug ticket has been created in the OpenJdk Jira. It is marked as reproduceable so most probably will be handled as bug, and fixed.

Here is the aforementioned ticket so we can trace the progress.

Edit: Seems that this issue is going to affect JDK 20 as well. Resolution is going to take some time.

Panagiotis Bougioukos
  • 15,955
  • 2
  • 30
  • 47
  • I have decided to accept the other answer as the mentioned issue currently also indicates that it is because of the specific way if else is done. – Fabian Kürten Jul 14 '22 at 10:55