Copilot commented on code in PR #2509:
URL: https://github.com/apache/groovy/pull/2509#discussion_r3177464384
##########
src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java:
##########
@@ -6645,6 +6653,38 @@ 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. A
+ * logical-AND breaks that: the else of {@code A && B} is reached when at
least one
+ * of {@code A}, {@code B} is false, not necessarily both — so an
instanceof inside
+ * either operand cannot be safely inverted. In particular, a {@code
!instanceof}
+ * inside an AND would otherwise be unwrapped into a positive smart-cast,
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();
+ if (op == LOGICAL_AND) return false;
+ if (op == LOGICAL_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());
+ }
Review Comment:
canInvertNarrowingForElseBranch only rejects LOGICAL_AND and only recurses
through LOGICAL_OR. In Groovy, boolean expressions can also involve
BITWISE_AND/BITWISE_XOR (and BITWISE_OR containing nested subexpressions), and
the same unsound inversion issue applies when the else branch can be reached
without falsifying every instanceof/!instanceof sub-check. Consider extending
this helper to traverse the full expression tree (including casts) and treat
additional boolean operators (at least BITWISE_AND, likely BITWISE_XOR; and
recurse into BITWISE_OR operands) so inversion is conservatively disabled
whenever an AND-like node appears anywhere in the condition.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
