7 ms·
negative instanceof is a disaster I posted a bit about it here http://www.benf.org/other/cfr/java14instanceof_pattern.html http://www.benf.org/other/cfr/java14
by leibnitz27 7y ago
negative instanceof is a disaster
I posted a bit about it here http://www.benf.org/other/cfr/java14instanceof_pattern.html http://www.benf.org/other/cfr/java14instanceof_pattern.html
it was first noted https://twitter.com/tagir_valeev/status/1210431331332689920 https://twitter.com/tagir_valeev/status/1210431331332689920 here
but the main thing is that if the 'taken' conditional is guaranteed to exit, then scope hiding happens, if not, not.
But in java
if (true) {
throw new Exception();
}
is not guaranteed to exit.
So: https://github.com/leibnitz27/cfr_tests/blob/master/src_14/org/benf/cfr/tests/InstanceOfPatternTest10.java https://github.com/leibnitz27/cfr_tests/blob/master/src_14/o...
In case you don't want to run, as of java (build 14-ea+34-1452)
this prints:
Fred
WIBBLE
public class InstanceOfPatternTest10 {
static String s = "WIBBLE";
public static void test(Object obj) {
if (!(obj instanceof String s)) {
throw new IllegalStateException();
}
System.out.println(s);
}
public static void test2(Object obj) {
if (!(obj instanceof String s)) {
if(true) {
throw new IllegalStateException();
}
}
System.out.println(s);
}
public static void main(String ... args) {
test("Fred");
test2("Fred");
}
}
- Jenz 7y agoOoops
- vbezhenar 7y agoLooks 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.
- leibnitz27 7y agoNope - the if condition is not considered in flow analysis. Read the end: https://docs.oracle.com/javase/specs/jls/se8/html/jls-14.html#jls-14.21 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."
- vbezhenar 7y agoI 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?
- leibnitz27 7y agoI haven't raised mine, as I consider it to be a refinement of the bug noted in https://twitter.com/tagir_valeev/status/1210431331332689920 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.
- vbezhenar 7y agoIMO 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.