RFR (14): JDK-8236670: Conflicting bindings accepted in some cases

[Jan Lahoda]

Hi,

Tagir reported a problem where javac accepts a program with conflicting 
bindings, like (simplified):

static void test(Object o1, Object o2) {
       boolean b = o1 instanceof String s && !(o2 instanceof String s));
}

There are two problem here:
-in the right-hand side of the conditional and, "s" is already in scope, 
but no error reported. This is because in Check, while checking clashes, 
clashes between binding variables are ignored, and expected to be 
reported in MatchBindingsComputer - but here, MatchBindingsComputer does 
not see the conflict when computing the binding variables for the 
expression. The proposed fix is to let the error be reported in Check, 
with some checks to avoid duplicate errors from MatchBindingsComputer. 
As a side effect, this should provide better error positions in some 
(although not all) cases, consider:
static void test(Object o1, Object o2) {
       if (o1 instanceof String s && (o2 instanceof String s));
}
before:
/tmp/T.java:3: error: illegal attempt to redefine an existing match binding
       if (o1 instanceof String s && (o2 instanceof String s));
                                  ^
with the patch:
/tmp/T.java:3: error: illegal attempt to redefine an existing match binding
       if (o1 instanceof String s && (o2 instanceof String s));
                                                           ^


-the second problem is more intricate - some bindings consistency 
constraints are curently only checked when match bindings are evaluated 
for a whole expression, like e.g.:
if (!(o1 instanceof String s) && !(o2 instanceof String s)) {}

this is illegal per JLS 6.3.1.1 ("It is a compile-time error if a 
pattern variable is both introduced by a when false, and by b when 
false.") The constraint is checked when the expression is e.g. a 
condition for an if statement, but not when the expression is e.g. an 
initializer to a variable:
boolean b = !(o1 instanceof String s) && !(o2 instanceof String s);

- this yields no error, as the expression is never checked by the 
MatchBindingComputer as a whole. We could either run the 
MatchBindingComputer on every non-conditional expression, but seems a 
nicer way is to build the current match bindings while attributing the 
expressions (i.e. keeping the match bindings in Attr and enhance/check 
them as it goes up the AST).

The current proposed patch is here:
http://cr.openjdk.java.net/~jlahoda/8236670/webrev.00/

JBS: https://bugs.openjdk.java.net/browse/JDK-8236670

What do you think?

Thanks,
      Jan