Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -4837,7 +4844,11 @@ public void visitTernaryExpression(final TernaryExpression expression) {
Map<Object, List<ClassNode>> 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();
Expand Down Expand Up @@ -6645,6 +6656,43 @@ private void putNotInstanceOfTypeInfo(final Object key, final Collection<ClassNo
tti.addAll(types); // stash negative type(s)
}

/**
* GROOVY-11983: Whether the temporary type info captured while visiting {@code expr}
* may be soundly inverted for the else branch of {@code if (expr) ... else ...} (or
* the false branch of a ternary, or the surrounding scope after an early-returning
* {@code if (expr) return}). Inversion is only sound when {@code !expr} pins down
* the negation of every individual instanceof / !instanceof check inside it.
* <p>
* 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 <em>at least one</em> 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());
}
Comment thread
paulk-asert marked this conversation as resolved.
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();
Expand Down
93 changes: 93 additions & 0 deletions src/test/groovy/groovy/transform/stc/TypeInferenceSTCTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Loading