diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java index 36a9c4d5e00..29392c4ed3c 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -56,6 +56,7 @@ import org.codehaus.groovy.ast.expr.AttributeExpression; import org.codehaus.groovy.ast.expr.BinaryExpression; import org.codehaus.groovy.ast.expr.BitwiseNegationExpression; +import org.codehaus.groovy.ast.expr.BooleanExpression; import org.codehaus.groovy.ast.expr.CastExpression; import org.codehaus.groovy.ast.expr.ClassExpression; import org.codehaus.groovy.ast.expr.ClosureExpression; @@ -254,6 +255,9 @@ import static org.codehaus.groovy.runtime.ArrayGroovyMethods.init; import static org.codehaus.groovy.runtime.ArrayGroovyMethods.last; import static org.codehaus.groovy.syntax.Types.ASSIGN; +import static org.codehaus.groovy.syntax.Types.BITWISE_AND; +import static org.codehaus.groovy.syntax.Types.BITWISE_OR; +import static org.codehaus.groovy.syntax.Types.BITWISE_XOR; import static org.codehaus.groovy.syntax.Types.COMPARE_EQUAL; import static org.codehaus.groovy.syntax.Types.COMPARE_NOT_EQUAL; import static org.codehaus.groovy.syntax.Types.COMPARE_NOT_IN; @@ -268,6 +272,7 @@ import static org.codehaus.groovy.syntax.Types.INTDIV_EQUAL; import static org.codehaus.groovy.syntax.Types.KEYWORD_IN; import static org.codehaus.groovy.syntax.Types.KEYWORD_INSTANCEOF; +import static org.codehaus.groovy.syntax.Types.LOGICAL_AND; import static org.codehaus.groovy.syntax.Types.LOGICAL_OR; import static org.codehaus.groovy.syntax.Types.MINUS_MINUS; import static org.codehaus.groovy.syntax.Types.MOD; @@ -4636,13 +4641,15 @@ public void visitIfElse(final IfStatement ifElse) { // GROOVY-6429: reverse instanceof tracking typeCheckingContext.pushTemporaryTypeInfo(); - tti.forEach(this::putNotInstanceOfTypeInfo); + // GROOVY-11983: only invert when the boolean's falsity pins down each instanceof + boolean canInvert = canInvertNarrowingForElseBranch(ifElse.getBooleanExpression()); + if (canInvert) tti.forEach(this::putNotInstanceOfTypeInfo); elsePath.visit(this); typeCheckingContext.popTemporaryTypeInfo(); // GROOVY-8523: propagate tracking to outer scope; keep simple for now - if (elsePath.isEmpty() && !GeneralUtils.maybeFallsThrough(thenPath)) { + if (canInvert && elsePath.isEmpty() && !GeneralUtils.maybeFallsThrough(thenPath)) { tti.forEach(this::putNotInstanceOfTypeInfo); } } finally { @@ -4837,7 +4844,11 @@ public void visitTernaryExpression(final TernaryExpression expression) { Map> tti = typeCheckingContext.temporaryIfBranchTypeInformation.pop(); typeCheckingContext.pushTemporaryTypeInfo(); - tti.forEach(this::putNotInstanceOfTypeInfo); // GROOVY-8412 + // GROOVY-8412 / GROOVY-11983: only invert when sound (no LOGICAL_AND in condition) + if (!(expression instanceof ElvisOperatorExpression) + && canInvertNarrowingForElseBranch(expression.getBooleanExpression())) { + tti.forEach(this::putNotInstanceOfTypeInfo); + } Expression falseExpression = expression.getFalseExpression(); ClassNode typeOfFalse = visitValueExpression(falseExpression); typeCheckingContext.popTemporaryTypeInfo(); @@ -6645,6 +6656,43 @@ private void putNotInstanceOfTypeInfo(final Object key, final Collection + * Only OR-like operators ({@code ||}, and {@code |} on booleans) preserve that + * property — {@code !(L op R)} is equivalent to {@code !L && !R}, so each operand's + * negation is pinned down. AND-like and XOR-like operators ({@code &&}, {@code &}, + * {@code ^}) do not: {@code !(L && R)} only requires at least one operand + * to be false, so a {@code !instanceof} hidden inside one of them would otherwise + * be unwrapped into a positive smart-cast in the else branch, producing an unsound + * {@code checkcast} and a runtime ClassCastException. + */ + private static boolean canInvertNarrowingForElseBranch(final Expression expr) { + if (expr instanceof BinaryExpression be) { + int op = be.getOperation().getType(); + // AND-like / XOR: !(L op R) doesn't pin down each operand's truth value + if (op == LOGICAL_AND || op == BITWISE_AND || op == BITWISE_XOR) return false; + // OR-like: !(L op R) <=> !L && !R, so recurse into both operands + if (op == LOGICAL_OR || op == BITWISE_OR) { + return canInvertNarrowingForElseBranch(be.getLeftExpression()) + && canInvertNarrowingForElseBranch(be.getRightExpression()); + } + return true; // instanceof, ==, comparisons, etc. + } + if (expr instanceof BooleanExpression be) return canInvertNarrowingForElseBranch(be.getExpression()); + if (expr instanceof NotExpression ne) return canInvertNarrowingForElseBranch(ne.getExpression()); + if (expr instanceof TernaryExpression te) { + return canInvertNarrowingForElseBranch(te.getBooleanExpression()) + && canInvertNarrowingForElseBranch(te.getTrueExpression()) + && canInvertNarrowingForElseBranch(te.getFalseExpression()); + } + return true; + } + private boolean optInstanceOfTypeInfo(final Expression expression, final ClassNode type) { var tti = typeCheckingContext.temporaryIfBranchTypeInformation.peek().get(extractTemporaryTypeInfoKey(expression)); if (tti != null) { assert !tti.isEmpty(); diff --git a/src/test/groovy/groovy/transform/stc/TypeInferenceSTCTest.groovy b/src/test/groovy/groovy/transform/stc/TypeInferenceSTCTest.groovy index 4a3ddf16da2..b833722f2c3 100644 --- a/src/test/groovy/groovy/transform/stc/TypeInferenceSTCTest.groovy +++ b/src/test/groovy/groovy/transform/stc/TypeInferenceSTCTest.groovy @@ -1031,6 +1031,99 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase { } } + // GROOVY-11983: !instanceof inside && must not produce a positive smart-cast in the else branch + @Test + void testNotInstanceof7() { + for (test in ['!(x instanceof String)', 'x !instanceof String']) { + // if/else form: a non-String x with cond=false lands in the else; if x were + // smart-cast to String, the body would emit a checkcast and throw CCE + assertScript """ + @groovy.transform.CompileStatic + class T { + static int f(Object x, boolean cond) { + if (cond && $test) { + return 1 + } else { + return x.hashCode() + } + } + } + assert T.f(42, false) == Integer.valueOf(42).hashCode() + assert T.f('s', false) == 's'.hashCode() + assert T.f('s', true) == 's'.hashCode() + assert T.f(42, true) == 1 + """ + // ternary form + assertScript """ + @groovy.transform.CompileStatic + class T { + static int g(Object x, boolean cond) { + return (cond && $test) ? 1 : x.hashCode() + } + } + assert T.g(42, false) == Integer.valueOf(42).hashCode() + assert T.g('s', false) == 's'.hashCode() + """ + // early-return surrounding-scope form + assertScript """ + @groovy.transform.CompileStatic + class T { + static int h(Object x, boolean cond) { + if (cond && $test) return 1 + return x.hashCode() + } + } + assert T.h(42, false) == Integer.valueOf(42).hashCode() + assert T.h('s', false) == 's'.hashCode() + """ + } + } + + // GROOVY-11983: bitwise & / ^ between booleans, and bitwise | wrapping a logical-and, + // must also disable the else-branch instanceof inversion + @Test + void testNotInstanceof8() { + // bitwise-AND: same unsoundness as logical-AND + assertScript ''' + @groovy.transform.CompileStatic + class T { + static int f(Object x, boolean cond) { + if (cond & !(x instanceof String)) return 1 + return x.hashCode() + } + } + assert T.f(42, false) == Integer.valueOf(42).hashCode() + assert T.f('s', false) == 's'.hashCode() + assert T.f('s', true) == 's'.hashCode() + assert T.f(42, true) == 1 + ''' + // bitwise-XOR: same unsoundness + assertScript ''' + @groovy.transform.CompileStatic + class T { + static int g(Object x, boolean cond) { + if (cond ^ (x instanceof String)) return 1 + return x.hashCode() + } + } + assert T.g(42, false) == Integer.valueOf(42).hashCode() + assert T.g('s', true) == 's'.hashCode() + ''' + // bitwise-OR wrapping a logical-AND: still must disqualify + assertScript ''' + @groovy.transform.CompileStatic + class T { + static int h(Object x, boolean a, boolean b) { + if (a | (b && !(x instanceof String))) return 1 + return x.hashCode() + } + } + assert T.h(42, false, false) == Integer.valueOf(42).hashCode() + assert T.h('s', false, false) == 's'.hashCode() + assert T.h(42, true, false) == 1 + ''' + } + // GROOVY-10217 @Test void testInstanceOfThenSubscriptOperator() {