You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2026/08/12 00:48:31 UTC
(groovy) 01/07: GROOVY-12242: instanceof pattern variable scope is not aligned with Java flow scoping (JEP 394)
This is an automated email from the ASF dual-hosted git repository.
paulk-asert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 087e4b76f9f7c6c996cb0d7c10f6569eeaee4715
Author: Daniel Sun <su...@apache.org>
AuthorDate: Sat Aug 8 14:08:03 2026 +0900
GROOVY-12242: instanceof pattern variable scope is not aligned with Java flow scoping (JEP 394)
---
.../groovy/ast/expr/DeclarationExpression.java | 11 +
.../groovy/classgen/InstanceofFlowBindings.java | 249 +++++++++++++++++++
.../groovy/classgen/VariableScopeVisitor.java | 111 ++++++++-
.../classgen/asm/BinaryExpressionHelper.java | 132 ++++++----
.../codehaus/groovy/classgen/asm/CompileStack.java | 25 ++
.../classgen/asm/InstanceofFlowSlotPublisher.java | 155 ++++++++++++
.../groovy/classgen/asm/StatementWriter.java | 56 ++++-
.../groovy/InstanceofFlowBindingsTest.groovy | 206 +++++++++++++++
src/test/groovy/groovy/InstanceofTest.groovy | 275 +++++++++++++++++++++
9 files changed, 1159 insertions(+), 61 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/ast/expr/DeclarationExpression.java b/src/main/java/org/codehaus/groovy/ast/expr/DeclarationExpression.java
index e4970bca8d..a7e6ec2aba 100644
--- a/src/main/java/org/codehaus/groovy/ast/expr/DeclarationExpression.java
+++ b/src/main/java/org/codehaus/groovy/ast/expr/DeclarationExpression.java
@@ -43,6 +43,12 @@ import static org.apache.groovy.ast.tools.ClassNodeUtils.formatTypeName;
* you can use the method "<code>TupleExpression getTupleExpression()</code>" method.
* Calling either of these expression getters when the "isMultipleAssignment" condition
* is not appropriate is unsafe and will result in a <code>ClassCastException</code>.
+ * <p>
+ * JEP 394 type patterns reuse this node as the right-hand side of
+ * {@code instanceof}: {@code e instanceof String s} is modelled as an
+ * {@code instanceof} {@link BinaryExpression} whose RHS is a
+ * {@code DeclarationExpression} with an {@link EmptyExpression} initializer.
+ * The pattern variable's scope is flow-sensitive (see GROOVY-12242).
*/
public class DeclarationExpression extends BinaryExpression {
@@ -130,6 +136,11 @@ public class DeclarationExpression extends BinaryExpression {
: null;
}
+ /**
+ * Returns the type of the declared variable (or of the tuple for a multiple
+ * assignment). For a JEP 394 pattern used as the RHS of {@code instanceof},
+ * this is the pattern type {@code T} in {@code e instanceof T t}.
+ */
@Override
public ClassNode getType() {
return (isMultipleAssignmentDeclaration() ? getTupleExpression() : getVariableExpression()).getType();
diff --git a/src/main/java/org/codehaus/groovy/classgen/InstanceofFlowBindings.java b/src/main/java/org/codehaus/groovy/classgen/InstanceofFlowBindings.java
new file mode 100644
index 0000000000..84e1d27a15
--- /dev/null
+++ b/src/main/java/org/codehaus/groovy/classgen/InstanceofFlowBindings.java
@@ -0,0 +1,249 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.groovy.classgen;
+
+import org.codehaus.groovy.ast.CodeVisitorSupport;
+import org.codehaus.groovy.ast.expr.BinaryExpression;
+import org.codehaus.groovy.ast.expr.BooleanExpression;
+import org.codehaus.groovy.ast.expr.DeclarationExpression;
+import org.codehaus.groovy.ast.expr.Expression;
+import org.codehaus.groovy.ast.expr.NotExpression;
+import org.codehaus.groovy.ast.expr.VariableExpression;
+import org.codehaus.groovy.syntax.Types;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+
+/**
+ * Flow-sensitive analysis of JEP 394 {@code instanceof} pattern bindings
+ * (GROOVY-12242).
+ * <p>
+ * This is pure <em>semantic</em> analysis: given a boolean expression, which
+ * pattern variables are <em>definitely bound</em> when the expression is
+ * {@code true} versus {@code false}? (Same idea as compiler “flow info” /
+ * JEP 394 flow scoping — not a bytecode construct.)
+ * <ul>
+ * <li>{@link #of(Expression)} — true/false binding sets for a condition</li>
+ * <li>{@link #containsPattern(Expression)} — nested type-pattern presence
+ * (e.g. whether an expression statement needs CompileStack isolation)</li>
+ * </ul>
+ * Covered shapes: {@code e instanceof T t}, negation / {@code !instanceof},
+ * {@code &&} (union of true bindings), {@code ||} (union of false bindings).
+ * Other shapes contribute nothing (conservative).
+ * <p>
+ * Consumers:
+ * <ul>
+ * <li>{@link VariableScopeVisitor} — declare names on the live path</li>
+ * <li>{@link org.codehaus.groovy.classgen.asm.InstanceofFlowSlotPublisher} —
+ * publish/hide CompileStack slots from these bindings</li>
+ * </ul>
+ *
+ * @see org.codehaus.groovy.classgen.asm.InstanceofFlowSlotPublisher
+ * @since 6.0.0
+ */
+public final class InstanceofFlowBindings {
+
+ private static final InstanceofFlowBindings EMPTY =
+ new InstanceofFlowBindings(List.of(), List.of());
+
+ private final List<VariableExpression> whenTrue;
+ private final List<VariableExpression> whenFalse;
+
+ private InstanceofFlowBindings(final List<VariableExpression> whenTrue,
+ final List<VariableExpression> whenFalse) {
+ this.whenTrue = whenTrue;
+ this.whenFalse = whenFalse;
+ }
+
+ /**
+ * Pattern variables that are definitely assigned when the analysed expression
+ * evaluates to {@code true}.
+ */
+ public List<VariableExpression> whenTrue() {
+ return whenTrue;
+ }
+
+ /**
+ * Pattern variables that are definitely assigned when the analysed expression
+ * evaluates to {@code false}.
+ */
+ public List<VariableExpression> whenFalse() {
+ return whenFalse;
+ }
+
+ /** Whether any pattern variable is bound on either path. */
+ public boolean isEmpty() {
+ return whenTrue.isEmpty() && whenFalse.isEmpty();
+ }
+
+ /**
+ * Names of pattern variables bound when the expression is {@code true}.
+ */
+ public Set<String> whenTrueNames() {
+ return names(whenTrue);
+ }
+
+ /**
+ * Names of pattern variables bound when the expression is {@code false}.
+ */
+ public Set<String> whenFalseNames() {
+ return names(whenFalse);
+ }
+
+ /**
+ * All pattern-variable names appearing in either path (stable encounter order).
+ */
+ public Set<String> allNames() {
+ if (isEmpty()) return Collections.emptySet();
+ Set<String> names = new LinkedHashSet<>(whenTrue.size() + whenFalse.size());
+ for (VariableExpression ve : whenTrue) names.add(ve.getName());
+ for (VariableExpression ve : whenFalse) names.add(ve.getName());
+ return names;
+ }
+
+ private static Set<String> names(final List<VariableExpression> vars) {
+ if (vars.isEmpty()) return Collections.emptySet();
+ Set<String> result = new LinkedHashSet<>(vars.size());
+ for (VariableExpression ve : vars) {
+ result.add(ve.getName());
+ }
+ return result;
+ }
+
+ /**
+ * Analyses {@code expression} for definite {@code instanceof} pattern bindings.
+ *
+ * @param expression a boolean condition (may be a {@link BooleanExpression} wrapper)
+ * @return the true/false binding sets; never {@code null}
+ */
+ public static InstanceofFlowBindings of(final Expression expression) {
+ if (expression == null) {
+ return EMPTY;
+ }
+ return analyse(expression);
+ }
+
+ /**
+ * Returns {@code true} if {@code expression} contains any JEP 394 type
+ * pattern ({@code e instanceof T t} or {@code e !instanceof T t}), including
+ * nested subexpressions. Used to decide whether expression-statement
+ * CompileStack isolation is required.
+ *
+ * @param expression any expression; {@code null} yields {@code false}
+ */
+ public static boolean containsPattern(final Expression expression) {
+ if (expression == null) return false;
+ boolean[] found = {false};
+ expression.visit(new CodeVisitorSupport() {
+ @Override
+ public void visitBinaryExpression(final BinaryExpression be) {
+ if (found[0]) return;
+ int op = be.getOperation().getType();
+ if ((op == Types.KEYWORD_INSTANCEOF || op == Types.COMPARE_NOT_INSTANCEOF)
+ && isTypePattern(be.getRightExpression())) {
+ found[0] = true;
+ return;
+ }
+ super.visitBinaryExpression(be);
+ }
+ });
+ return found[0];
+ }
+
+ private static boolean isTypePattern(final Expression right) {
+ return right instanceof DeclarationExpression decl
+ && !decl.isMultipleAssignmentDeclaration()
+ && decl.getVariableExpression() != null;
+ }
+
+ private static InstanceofFlowBindings analyse(final Expression expression) {
+ Expression expr = expression;
+
+ // Unwrap BooleanExpression wrappers; NotExpression is handled below so that
+ // nested negations compose correctly.
+ while (expr instanceof BooleanExpression && !(expr instanceof NotExpression)) {
+ expr = ((BooleanExpression) expr).getExpression();
+ }
+
+ if (expr instanceof NotExpression not) {
+ return analyse(not.getExpression()).negated();
+ }
+
+ if (expr instanceof BinaryExpression binary) {
+ int op = binary.getOperation().getType();
+ if (op == Types.KEYWORD_INSTANCEOF) {
+ return ofInstanceof(binary);
+ }
+ if (op == Types.COMPARE_NOT_INSTANCEOF) {
+ // AST may still carry !instanceof before codegen rewrites it to !(… instanceof …).
+ return ofInstanceof(binary).negated();
+ }
+ if (op == Types.LOGICAL_AND) {
+ InstanceofFlowBindings left = analyse(binary.getLeftExpression());
+ InstanceofFlowBindings right = analyse(binary.getRightExpression());
+ // True path evaluates both; false path is not definite for either side alone.
+ return new InstanceofFlowBindings(
+ union(left.whenTrue, right.whenTrue),
+ List.of());
+ }
+ if (op == Types.LOGICAL_OR) {
+ InstanceofFlowBindings left = analyse(binary.getLeftExpression());
+ InstanceofFlowBindings right = analyse(binary.getRightExpression());
+ // False path evaluates both; true path is not definite for either side alone.
+ return new InstanceofFlowBindings(
+ List.of(),
+ union(left.whenFalse, right.whenFalse));
+ }
+ }
+
+ return EMPTY;
+ }
+
+ private static InstanceofFlowBindings ofInstanceof(final BinaryExpression binary) {
+ Expression right = binary.getRightExpression();
+ if (isTypePattern(right)) {
+ VariableExpression patternVar = ((DeclarationExpression) right).getVariableExpression();
+ return new InstanceofFlowBindings(List.of(patternVar), List.of());
+ }
+ return EMPTY;
+ }
+
+ private InstanceofFlowBindings negated() {
+ if (isEmpty()) return this;
+ return new InstanceofFlowBindings(whenFalse, whenTrue);
+ }
+
+ private static List<VariableExpression> union(final List<VariableExpression> a,
+ final List<VariableExpression> b) {
+ if (a.isEmpty()) return b;
+ if (b.isEmpty()) return a;
+ List<VariableExpression> result = new ArrayList<>(a.size() + b.size());
+ Set<String> seen = new LinkedHashSet<>();
+ for (VariableExpression ve : a) {
+ if (seen.add(ve.getName())) result.add(ve);
+ }
+ for (VariableExpression ve : b) {
+ if (seen.add(ve.getName())) result.add(ve);
+ }
+ return List.copyOf(result);
+ }
+}
diff --git a/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java b/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
index b9c7126c37..c2acdcadfd 100644
--- a/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
@@ -45,6 +45,7 @@ import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.FieldExpression;
import org.codehaus.groovy.ast.expr.MethodCallExpression;
import org.codehaus.groovy.ast.expr.PropertyExpression;
+import org.codehaus.groovy.ast.expr.TernaryExpression;
import org.codehaus.groovy.ast.expr.TupleExpression;
import org.codehaus.groovy.ast.expr.VariableExpression;
import org.codehaus.groovy.ast.stmt.AssertStatement;
@@ -66,6 +67,7 @@ import java.util.Deque;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedList;
+import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.function.BiConsumer;
@@ -76,10 +78,17 @@ import static java.lang.reflect.Modifier.isStatic;
import static org.apache.groovy.ast.tools.MethodNodeUtils.getPropertyName;
import static org.apache.groovy.ast.tools.MethodNodeUtils.withDefaultArgumentMethods;
import static org.codehaus.groovy.ast.tools.GeneralUtils.getAllProperties;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.maybeFallsThrough;
import static org.codehaus.groovy.transform.trait.Traits.isTrait;
/**
* Initializes the variable scopes for an AST.
+ * <p>
+ * For JEP 394 {@code instanceof} pattern variables (GROOVY-12242), name
+ * resolution follows {@link InstanceofFlowBindings} (flow analysis): a pattern
+ * variable is only declared where the pattern has definitely matched. Bytecode
+ * slot visibility applies the same analysis via
+ * {@link org.codehaus.groovy.classgen.asm.InstanceofFlowSlotPublisher}.
*/
public class VariableScopeVisitor extends ClassCodeVisitorSupport {
@@ -623,20 +632,45 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
}
/**
- * {@inheritDoc}
+ * Visits an {@code if}/{@code else} with Java-aligned flow scoping for
+ * {@code instanceof} pattern variables (GROOVY-12242 / JEP 394).
+ * <p>
+ * Pattern variables that bind when the condition is {@code true} are in
+ * scope in the then-block; those that bind when it is {@code false} are in
+ * scope in the else-block. When a branch cannot complete normally, bindings
+ * from the opposite path remain in scope for subsequent statements (e.g.
+ * {@code if (!(o instanceof String s)) return; s.length()}).
*/
@Override
public void visitIfElse(final IfStatement statement) {
+ InstanceofFlowBindings bindings = InstanceofFlowBindings.of(statement.getBooleanExpression());
+
+ // Condition: pattern vars are available for short-circuit RHS (e.g. &&).
pushState();
visitStatement(statement);
statement.getBooleanExpression().visit(this);
+ popState();
+
+ // Then-block: only true-path bindings.
pushState();
+ declarePatternVariables(bindings.whenTrue());
statement.getIfBlock().visit(this);
popState();
- popState();
+
+ // Else-block: only false-path bindings.
pushState();
+ declarePatternVariables(bindings.whenFalse());
statement.getElseBlock().visit(this);
popState();
+
+ // After the if: Java keeps the opposite path's bindings when a branch
+ // cannot complete normally (early return / throw).
+ if (!maybeFallsThrough(statement.getIfBlock())) {
+ declarePatternVariables(bindings.whenFalse());
+ }
+ if (!statement.getElseBlock().isEmpty() && !maybeFallsThrough(statement.getElseBlock())) {
+ declarePatternVariables(bindings.whenTrue());
+ }
}
/**
@@ -660,7 +694,9 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
}
/**
- * {@inheritDoc}
+ * Visits a {@code while} loop. Pattern variables introduced by the condition
+ * are in scope for the condition's short-circuit RHS and for the loop body
+ * (GROOVY-12242). They do not leak past the loop.
*/
@Override
public void visitWhileLoop(final WhileStatement statement) {
@@ -669,6 +705,20 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
popState();
}
+ /**
+ * Declares {@code instanceof} pattern variables into the current scope so
+ * that subsequent visits resolve them as locals. Skips names already present
+ * in this scope (re-declaring the same pattern variable object after a
+ * condition visit is a no-op).
+ */
+ private void declarePatternVariables(final List<VariableExpression> patternVariables) {
+ for (VariableExpression variable : patternVariables) {
+ if (currentScope.getDeclaredVariable(variable.getName()) == null) {
+ declare(variable);
+ }
+ }
+ }
+
// expressions:
/**
@@ -681,17 +731,66 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
}
/**
- * {@inheritDoc}
+ * Visits binary expressions with flow-aware scoping for {@code &&} / {@code ||}
+ * so pattern variables follow Java short-circuit rules (GROOVY-12242 / JEP 394):
+ * <ul>
+ * <li>{@code a && b} — true-path bindings of {@code a} are in scope in {@code b}</li>
+ * <li>{@code a || b} — true-path bindings of {@code a} are <em>not</em> in scope in {@code b};
+ * false-path bindings of {@code a} are</li>
+ * </ul>
*/
@Override
public void visitBinaryExpression(final BinaryExpression expression) {
- super.visitBinaryExpression(expression);
+ int op = expression.getOperation().getType();
+ if (op == Types.LOGICAL_AND) {
+ // Left first; its true-path pattern vars stay in the current scope for the right.
+ expression.getLeftExpression().visit(this);
+ expression.getRightExpression().visit(this);
+ } else if (op == Types.LOGICAL_OR) {
+ // Left's true-path bindings must not leak into the right (Java rejects
+ // `o instanceof String s || s.isEmpty()`). False-path bindings of the
+ // left are in scope on the right (`!(o instanceof String s) || s.isEmpty()`).
+ InstanceofFlowBindings leftBindings = InstanceofFlowBindings.of(expression.getLeftExpression());
+ pushState();
+ expression.getLeftExpression().visit(this);
+ popState();
+ pushState();
+ declarePatternVariables(leftBindings.whenFalse());
+ expression.getRightExpression().visit(this);
+ popState();
+ } else {
+ super.visitBinaryExpression(expression);
+ }
- if (Types.isAssignment(expression.getOperation().getType())) {
+ if (Types.isAssignment(op)) {
checkFinalFieldAccess(expression.getLeftExpression());
}
}
+ /**
+ * Visits a ternary / Elvis expression with flow scoping for pattern variables:
+ * true-path bindings are in scope in the then-branch; false-path bindings in
+ * the else-branch (GROOVY-12242 / JEP 394).
+ */
+ @Override
+ public void visitTernaryExpression(final TernaryExpression expression) {
+ InstanceofFlowBindings bindings = InstanceofFlowBindings.of(expression.getBooleanExpression());
+
+ pushState();
+ expression.getBooleanExpression().visit(this);
+ popState();
+
+ pushState();
+ declarePatternVariables(bindings.whenTrue());
+ expression.getTrueExpression().visit(this);
+ popState();
+
+ pushState();
+ declarePatternVariables(bindings.whenFalse());
+ expression.getFalseExpression().visit(this);
+ popState();
+ }
+
/**
* {@inheritDoc}
*/
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java b/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java
index 01c3e3dac3..951168493e 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java
@@ -47,6 +47,7 @@ import org.codehaus.groovy.ast.tools.GenericsUtils;
import org.codehaus.groovy.ast.tools.WideningCategories;
import org.codehaus.groovy.classgen.AsmClassGenerator;
import org.codehaus.groovy.classgen.BytecodeExpression;
+import org.codehaus.groovy.classgen.InstanceofFlowBindings;
import org.codehaus.groovy.runtime.MultipleAssignmentSupport;
import org.codehaus.groovy.runtime.ScriptBytecodeAdapter;
import org.codehaus.groovy.syntax.Token;
@@ -565,47 +566,44 @@ public class BinaryExpressionHelper {
}
// evaluate RHS and store its value
- if (lhsType.isArray() && rightExpression instanceof ListExpression) { // array = [ ... ]
- Expression array = new ArrayExpression(lhsType.getComponentType(), ((ListExpression) rightExpression).getExpressions());
- array.setSourcePosition(rightExpression);
- array.setType(lhsType);
- array.visit(acg);
- } else if (rightExpression instanceof EmptyExpression) { // define field
- CompileStack.pushInitValue(lhsType, mv);
- operandStack.push(lhsType);
- } else {
- rightExpression.visit(acg);
- }
-
- ClassNode rhsType = operandStack.getTopOperand();
-
+ // GROOVY-12242: for a single-variable declaration, evaluate the RHS in a nested
+ // CompileStack state so instanceof pattern variables do not leak past the
+ // declaration while the LHS local is defined in the outer state.
if (directAssignment) {
- VariableExpression var = (VariableExpression) leftExpression;
- if (var.isClosureSharedVariable() && ClassHelper.isPrimitiveType(rhsType)) {
- // GROOVY-5570: if a closure shared variable is a primitive type, it must be boxed
- rhsType = ClassHelper.getWrapper(rhsType);
- operandStack.box();
- }
+ compileStack.pushState();
+ try {
+ evaluateRightHandSide(lhsType, rightExpression, acg, mv, operandStack);
+ ClassNode rhsType = operandStack.getTopOperand();
+ VariableExpression var = (VariableExpression) leftExpression;
+ if (var.isClosureSharedVariable() && ClassHelper.isPrimitiveType(rhsType)) {
+ // GROOVY-5570: if a closure shared variable is a primitive type, it must be boxed
+ rhsType = ClassHelper.getWrapper(rhsType);
+ operandStack.box();
+ }
- // ensure we try to unbox null to cause a runtime NPE in case we assign
- // null to a primitive typed variable, even if it is used only in boxed
- // form as it is closure shared
- if (var.isClosureSharedVariable() && ClassHelper.isPrimitiveType(var.getOriginType()) && isNullConstant(rightExpression)) {
- operandStack.doGroovyCast(var.getOriginType());
- // these two are never reached in bytecode and only there
- // to avoid verify errors and compiler infrastructure hazzle
- operandStack.box();
- operandStack.doGroovyCast(lhsType);
- }
- // normal type transformation
- if (!ClassHelper.isPrimitiveType(lhsType) && isNullConstant(rightExpression)) {
- operandStack.replace(lhsType);
- } else {
- operandStack.doGroovyCast(lhsType);
+ // ensure we try to unbox null to cause a runtime NPE in case we assign
+ // null to a primitive typed variable, even if it is used only in boxed
+ // form as it is closure shared
+ if (var.isClosureSharedVariable() && ClassHelper.isPrimitiveType(var.getOriginType()) && isNullConstant(rightExpression)) {
+ operandStack.doGroovyCast(var.getOriginType());
+ // these two are never reached in bytecode and only there
+ // to avoid verify errors and compiler infrastructure hazzle
+ operandStack.box();
+ operandStack.doGroovyCast(lhsType);
+ }
+ // normal type transformation
+ if (!ClassHelper.isPrimitiveType(lhsType) && isNullConstant(rightExpression)) {
+ operandStack.replace(lhsType);
+ } else {
+ operandStack.doGroovyCast(lhsType);
+ }
+ } finally {
+ // Drop RHS pattern locals before defining the LHS in the outer state
+ compileStack.pop();
}
// store value
- BytecodeVariable v = compileStack.defineVariable(var, lhsType, true);
+ BytecodeVariable v = compileStack.defineVariable((Variable) leftExpression, lhsType, true);
operandStack.remove(1);
if (returnRightValue) {
new VariableSlotLoader(lhsType, v.getIndex(), operandStack).visit(acg);
@@ -613,6 +611,9 @@ public class BinaryExpressionHelper {
return;
}
+ evaluateRightHandSide(lhsType, rightExpression, acg, mv, operandStack);
+ ClassNode rhsType = operandStack.getTopOperand();
+
// GROOVY-10918: direct store to local variable or parameter (no temp)
if (!defineVariable && leftExpression instanceof VariableExpression) {
BytecodeVariable v = compileStack.getVariable(leftExpression.getText(), false);
@@ -1110,6 +1111,37 @@ public class BinaryExpressionHelper {
controller.getCompileStack().popLHS();
}
+ /**
+ * Evaluates the right-hand side of an assignment onto the operand stack.
+ */
+ private void evaluateRightHandSide(final ClassNode lhsType, final Expression rightExpression,
+ final AsmClassGenerator acg, final MethodVisitor mv,
+ final OperandStack operandStack) {
+ if (lhsType.isArray() && rightExpression instanceof ListExpression) { // array = [ ... ]
+ Expression array = new ArrayExpression(lhsType.getComponentType(), ((ListExpression) rightExpression).getExpressions());
+ array.setSourcePosition(rightExpression);
+ array.setType(lhsType);
+ array.visit(acg);
+ } else if (rightExpression instanceof EmptyExpression) { // define field
+ CompileStack.pushInitValue(lhsType, mv);
+ operandStack.push(lhsType);
+ } else {
+ rightExpression.visit(acg);
+ }
+ }
+
+ /**
+ * Emits bytecode for {@code e instanceof T} and, when the right-hand side is
+ * a JEP 394 type pattern ({@code e instanceof T t}), conditionally stores
+ * the checked value into the pattern variable {@code t}.
+ * <p>
+ * The pattern variable's visibility is governed by flow scoping in
+ * {@link org.codehaus.groovy.classgen.VariableScopeVisitor} and
+ * {@link StatementWriter#writeIfElse}; this method only performs the store.
+ *
+ * @param expression an {@code instanceof} binary expression
+ * @see org.codehaus.groovy.classgen.InstanceofFlowBindings
+ */
private void evaluateInstanceof(final BinaryExpression expression) {
CompileStack compileStack = controller.getCompileStack();
OperandStack operandStack = controller.getOperandStack();
@@ -1117,25 +1149,24 @@ public class BinaryExpressionHelper {
expression.getLeftExpression().visit(controller.getAcg());
operandStack.box(); // TODO: support instanceof primitives
- //ClassNode sourceType = operandStack.getTopOperand();
ClassNode targetType = expression.getRightExpression().getType();
-
- var jep394 = !(expression.getRightExpression() instanceof ClassExpression);
- if (jep394) {
- operandStack.dup(); // stash value for use by JEP 394 pattern variable
+ // JEP 394: RHS is DeclarationExpression (Type name) rather than ClassExpression
+ boolean patternMatch = !(expression.getRightExpression() instanceof ClassExpression);
+ if (patternMatch) {
+ operandStack.dup(); // stash value for the pattern variable store
}
String typeName = BytecodeHelper.getClassInternalName(targetType);
controller.getMethodVisitor().visitTypeInsn(INSTANCEOF, typeName);
operandStack.replace(ClassHelper.boolean_TYPE);
- if (jep394) {
+ if (patternMatch) {
var variable = (Variable) ((BinaryExpression) expression.getRightExpression()).getLeftExpression();
BytecodeVariable v = compileStack.defineVariable(variable, targetType, false);
MethodVisitor mv = controller.getMethodVisitor();
mv.visitInsn(DUP_X1); // stack: ..., check, value, check
- Label l0 = operandStack.jump(IFEQ); // skip store if not instanceof
+ Label notInstance = operandStack.jump(IFEQ); // skip store if not instanceof
mv.visitTypeInsn(CHECKCAST, typeName);
if (!v.isHolder()) {
@@ -1146,12 +1177,12 @@ public class BinaryExpressionHelper {
mv.visitInsn(SWAP);
mv.visitMethodInsn(INVOKEVIRTUAL, "groovy/lang/Reference", "set", "(Ljava/lang/Object;)V", false);
}
- Label l1 = operandStack.jump(GOTO);
+ Label done = operandStack.jump(GOTO);
- mv.visitLabel(l0); // stack: ..., check, value
+ mv.visitLabel(notInstance); // stack: ..., check, value
mv.visitInsn(POP);
- mv.visitLabel(l1); // stack: ..., check
+ mv.visitLabel(done); // stack: ..., check
operandStack.push(ClassHelper.boolean_TYPE);
}
}
@@ -1441,23 +1472,30 @@ public class BinaryExpressionHelper {
ClassNode commonType = WideningCategories.lowestUpperBound(truePartType, falsePartType);
// write "x?y:z" as "x?T(y):T(z)" where T is common type of y and z
+ CompileStack compileStack = controller.getCompileStack();
OperandStack operandStack = controller.getOperandStack();
MethodVisitor mv = controller.getMethodVisitor();
- // load x
+ // load x; hide pattern locals then publish only on the live arm (GROOVY-12242)
boolPart.visit(controller.getAcg());
+ InstanceofFlowSlotPublisher slotPublisher = InstanceofFlowSlotPublisher.captureAndHide(
+ compileStack, InstanceofFlowBindings.of(expression.getBooleanExpression()));
Label l0 = operandStack.jump(IFEQ);
// true path: load y and cast to T
+ slotPublisher.publishTrue(compileStack);
truePart.visit(controller.getAcg());
operandStack.doGroovyCast(commonType);
+ slotPublisher.hideTrue(compileStack);
Label l1 = new Label();
mv.visitJumpInsn(GOTO, l1);
// false path: load z and cast to T
mv.visitLabel(l0);
+ slotPublisher.publishFalse(compileStack);
falsePart.visit(controller.getAcg());
operandStack.doGroovyCast(commonType);
+ slotPublisher.hideFalse(compileStack);
// finish up
mv.visitLabel(l1);
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/CompileStack.java b/src/main/java/org/codehaus/groovy/classgen/asm/CompileStack.java
index 796617f32f..5854e07ac0 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/CompileStack.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/CompileStack.java
@@ -908,6 +908,31 @@ public class CompileStack {
return stackVariables.containsKey(name);
}
+ /**
+ * Re-publishes a previously defined local into the current state.
+ * Used by {@link InstanceofFlowSlotPublisher} for path-scoped pattern variables
+ * (GROOVY-12242 / JEP 394).
+ *
+ * @param variable the bytecode variable to make visible again
+ */
+ public void putVariable(final BytecodeVariable variable) {
+ if (variable != null) {
+ stackVariables.put(variable.getName(), variable);
+ }
+ }
+
+ /**
+ * Removes a named local from the current state without affecting temporary
+ * variables or the free-register cursor. Used by {@link InstanceofFlowSlotPublisher}
+ * to hide pattern slots that are not live on the current control-flow path.
+ *
+ * @param name the variable name to remove
+ * @return the removed variable, or {@code null} if it was not present
+ */
+ public BytecodeVariable removeVariable(final String name) {
+ return stackVariables.remove(name);
+ }
+
/**
* Calculates the index of the next free register stores it
* and sets the current variable index to the old value
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/InstanceofFlowSlotPublisher.java b/src/main/java/org/codehaus/groovy/classgen/asm/InstanceofFlowSlotPublisher.java
new file mode 100644
index 0000000000..f2f875227f
--- /dev/null
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/InstanceofFlowSlotPublisher.java
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.groovy.classgen.asm;
+
+import org.codehaus.groovy.classgen.InstanceofFlowBindings;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Codegen helper that publishes/hides {@link CompileStack} local <em>slots</em>
+ * for {@code instanceof} pattern variables according to
+ * {@link InstanceofFlowBindings} (GROOVY-12242 / JEP 394).
+ * <p>
+ * Distinct from {@link InstanceofFlowBindings}: that type is AST flow
+ * <em>analysis</em>; this type is bytecode <em>slot control</em>.
+ * <p>
+ * {@code evaluateInstanceof} defines pattern slots while the condition runs
+ * (needed for short-circuit {@code &&} RHS). After the condition, this helper
+ * <em>hides</em> every captured slot and then <em>publishes</em> only names
+ * live on the current control-flow path:
+ * <ul>
+ * <li>then-block → {@link InstanceofFlowBindings#whenTrueNames()}</li>
+ * <li>else-block → {@link InstanceofFlowBindings#whenFalseNames()}</li>
+ * <li>after the if → opposite path when a branch cannot complete normally</li>
+ * </ul>
+ * That “hide all, publish path” rule keeps CompileStack polarity aligned with
+ * {@link org.codehaus.groovy.classgen.VariableScopeVisitor}.
+ *
+ * @see InstanceofFlowBindings
+ * @since 6.0.0
+ */
+final class InstanceofFlowSlotPublisher {
+
+ private static final InstanceofFlowSlotPublisher NONE =
+ new InstanceofFlowSlotPublisher(InstanceofFlowBindings.of(null), Map.of());
+
+ private final InstanceofFlowBindings bindings;
+ private final Map<String, BytecodeVariable> captured;
+
+ private InstanceofFlowSlotPublisher(final InstanceofFlowBindings bindings,
+ final Map<String, BytecodeVariable> captured) {
+ this.bindings = bindings;
+ this.captured = captured;
+ }
+
+ /**
+ * Snapshots pattern slots defined while evaluating {@code bindings}' condition
+ * and removes them from {@code compileStack} so no branch sees unscoped slots.
+ *
+ * @param compileStack current compile stack (condition already evaluated)
+ * @param bindings flow-analysis result for that condition
+ * @return a publisher for path-scoped reintroduction of the captured slots
+ */
+ static InstanceofFlowSlotPublisher captureAndHide(final CompileStack compileStack,
+ final InstanceofFlowBindings bindings) {
+ if (bindings == null || bindings.isEmpty()) {
+ return NONE;
+ }
+ Map<String, BytecodeVariable> captured = new HashMap<>();
+ for (String name : bindings.allNames()) {
+ BytecodeVariable bv = compileStack.getVariable(name, false);
+ if (bv != null) {
+ captured.put(name, bv);
+ compileStack.removeVariable(name);
+ }
+ }
+ if (captured.isEmpty()) {
+ return NONE;
+ }
+ return new InstanceofFlowSlotPublisher(bindings, Collections.unmodifiableMap(captured));
+ }
+
+ boolean isEmpty() {
+ return captured.isEmpty();
+ }
+
+ /** Makes true-path pattern locals visible on the current CompileStack frame. */
+ void publishTrue(final CompileStack compileStack) {
+ publish(compileStack, bindings.whenTrueNames());
+ }
+
+ /** Makes false-path pattern locals visible on the current CompileStack frame. */
+ void publishFalse(final CompileStack compileStack) {
+ publish(compileStack, bindings.whenFalseNames());
+ }
+
+ /** Hides true-path pattern locals (end of then-block). */
+ void hideTrue(final CompileStack compileStack) {
+ hide(compileStack, bindings.whenTrueNames());
+ }
+
+ /** Hides false-path pattern locals (end of else-block). */
+ void hideFalse(final CompileStack compileStack) {
+ hide(compileStack, bindings.whenFalseNames());
+ }
+
+ /**
+ * Publishes bindings that remain in scope after the if, matching Java's
+ * abrupt-completion rule: opposite-path bindings survive when a branch
+ * cannot complete normally.
+ *
+ * @param ifFallsThrough whether the then-block may complete normally
+ * @param elseEmpty whether there is no else branch
+ * @param elseFallsThrough whether the else-block may complete normally
+ */
+ void publishAfterIf(final CompileStack compileStack,
+ final boolean ifFallsThrough,
+ final boolean elseEmpty,
+ final boolean elseFallsThrough) {
+ if (!ifFallsThrough) {
+ publishFalse(compileStack);
+ }
+ if (!elseEmpty && !elseFallsThrough) {
+ publishTrue(compileStack);
+ }
+ }
+
+ private void publish(final CompileStack compileStack, final Set<String> names) {
+ if (captured.isEmpty() || names.isEmpty()) return;
+ for (String name : names) {
+ BytecodeVariable bv = captured.get(name);
+ if (bv != null) {
+ compileStack.putVariable(bv);
+ }
+ }
+ }
+
+ private void hide(final CompileStack compileStack, final Set<String> names) {
+ if (captured.isEmpty() || names.isEmpty()) return;
+ for (String name : names) {
+ if (captured.containsKey(name)) {
+ compileStack.removeVariable(name);
+ }
+ }
+ }
+}
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
index 2154f9ef00..f5af5e83cc 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
@@ -23,6 +23,7 @@ import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.VariableScope;
import org.codehaus.groovy.ast.expr.BinaryExpression;
import org.codehaus.groovy.ast.expr.ClosureListExpression;
+import org.codehaus.groovy.ast.expr.DeclarationExpression;
import org.codehaus.groovy.ast.expr.EmptyExpression;
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.MethodCall;
@@ -45,6 +46,7 @@ import org.codehaus.groovy.ast.stmt.ThrowStatement;
import org.codehaus.groovy.ast.stmt.TryCatchStatement;
import org.codehaus.groovy.ast.stmt.WhileStatement;
import org.codehaus.groovy.classgen.AsmClassGenerator;
+import org.codehaus.groovy.classgen.InstanceofFlowBindings;
import org.codehaus.groovy.classgen.asm.CompileStack.BlockRecorder;
import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
@@ -470,6 +472,11 @@ public class StatementWriter {
/**
* Generates bytecode for an if/else statement.
+ * <p>
+ * GROOVY-12242 / JEP 394: after the condition runs, pattern locals are hidden
+ * then re-published only on the live path (then / else / after abrupt
+ * completion) via {@link InstanceofFlowSlotPublisher}, matching
+ * {@link org.codehaus.groovy.classgen.VariableScopeVisitor}.
*
* @param statement the if statement to compile
*/
@@ -477,22 +484,38 @@ public class StatementWriter {
controller.getAcg().onLineNumber(statement, "visitIfElse");
writeStatementLabel(statement);
- Label exitPath = controller.getCompileStack().pushBreakable(statement.getStatementLabels()); // GROOVY-7463
+ CompileStack compileStack = controller.getCompileStack();
+ InstanceofFlowBindings bindings = InstanceofFlowBindings.of(statement.getBooleanExpression());
+
+ Label exitPath = compileStack.pushBreakable(statement.getStatementLabels()); // GROOVY-7463
statement.getBooleanExpression().visit(controller.getAcg());
+ // Hide every pattern slot; publish only path-live bindings below.
+ InstanceofFlowSlotPublisher slotPublisher = InstanceofFlowSlotPublisher.captureAndHide(compileStack, bindings);
+
Label elsePath = controller.getOperandStack().jump(IFEQ);
+ slotPublisher.publishTrue(compileStack);
statement.getIfBlock().visit(controller.getAcg());
- controller.getCompileStack().pop();
+ slotPublisher.hideTrue(compileStack);
+ compileStack.pop(); // ends breakable
+
+ boolean ifFallsThrough = maybeFallsThrough(statement.getIfBlock());
+ boolean elseEmpty = statement.getElseBlock().isEmpty();
+ boolean elseFallsThrough = elseEmpty || maybeFallsThrough(statement.getElseBlock());
MethodVisitor mv = controller.getMethodVisitor();
- if (statement.getElseBlock().isEmpty()) {
+ if (elseEmpty) {
mv.visitLabel(elsePath);
} else {
- if (maybeFallsThrough(statement.getIfBlock())) {
+ if (ifFallsThrough) {
mv.visitJumpInsn(GOTO, exitPath);
}
mv.visitLabel(elsePath);
+ slotPublisher.publishFalse(compileStack);
statement.getElseBlock().visit(controller.getAcg());
+ slotPublisher.hideFalse(compileStack);
}
+
+ slotPublisher.publishAfterIf(compileStack, ifFallsThrough, elseEmpty, elseFallsThrough);
mv.visitLabel(exitPath);
}
@@ -883,6 +906,10 @@ public class StatementWriter {
* Evaluates the expression and discards any value left on the operand stack.
* Marks method-call and binary expressions so that unused return values
* are elided rather than boxed.
+ * <p>
+ * GROOVY-12242: non-declaration statements that contain an {@code instanceof}
+ * type pattern run in a nested CompileStack state so pattern locals cannot
+ * leak. Detection uses {@link InstanceofFlowBindings#containsPattern}.
*
* @param statement the expression statement to compile
*/
@@ -895,9 +922,22 @@ public class StatementWriter {
if (expression instanceof MethodCall || expression instanceof BinaryExpression)
expression.putNodeMetaData(AsmClassGenerator.ELIDE_EXPRESSION_VALUE, Boolean.TRUE);
- var operandStack = controller.getOperandStack();
- int mark = operandStack.getStackLength();
- expression.visit(controller.getAcg());
- operandStack.popDownTo(mark);
+ CompileStack compileStack = controller.getCompileStack();
+ // Declaration LHS isolation is in evaluateEqual; multi-assign must not be wrapped.
+ boolean isolatesPatternVars = !(expression instanceof DeclarationExpression)
+ && InstanceofFlowBindings.containsPattern(expression);
+ if (isolatesPatternVars) {
+ compileStack.pushState();
+ }
+ try {
+ var operandStack = controller.getOperandStack();
+ int mark = operandStack.getStackLength();
+ expression.visit(controller.getAcg());
+ operandStack.popDownTo(mark);
+ } finally {
+ if (isolatesPatternVars) {
+ compileStack.pop();
+ }
+ }
}
}
diff --git a/src/test/groovy/groovy/InstanceofFlowBindingsTest.groovy b/src/test/groovy/groovy/InstanceofFlowBindingsTest.groovy
new file mode 100644
index 0000000000..72a4d61aff
--- /dev/null
+++ b/src/test/groovy/groovy/InstanceofFlowBindingsTest.groovy
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package groovy
+
+import org.codehaus.groovy.ast.DynamicVariable
+import org.codehaus.groovy.ast.expr.Expression
+import org.codehaus.groovy.ast.expr.VariableExpression
+import org.codehaus.groovy.ast.stmt.IfStatement
+import org.codehaus.groovy.classgen.InstanceofFlowBindings
+import org.codehaus.groovy.control.CompilationUnit
+import org.codehaus.groovy.control.Phases
+import org.codehaus.groovy.ast.CodeVisitorSupport
+import org.junit.jupiter.api.Test
+
+final class InstanceofFlowBindingsTest {
+
+ @Test
+ void testBindingsOfPositiveInstanceof() {
+ def b = InstanceofFlowBindings.of(parseCondition('o instanceof String s'))
+ assert b.whenTrue()*.name == ['s']
+ assert b.whenFalse().isEmpty()
+ assert b.allNames() as List == ['s']
+ }
+
+ @Test
+ void testBindingsOfNegatedInstanceof() {
+ def b = InstanceofFlowBindings.of(parseCondition('!(o instanceof String s)'))
+ assert b.whenTrue().isEmpty()
+ assert b.whenFalse()*.name == ['s']
+ assert b.allNames() as List == ['s']
+ }
+
+ @Test
+ void testBindingsOfAnd() {
+ def b = InstanceofFlowBindings.of(parseCondition('o instanceof String s && s.length() > 0'))
+ assert b.whenTrue()*.name == ['s']
+ assert b.whenFalse().isEmpty()
+ }
+
+ @Test
+ void testBindingsOfOr() {
+ def b = InstanceofFlowBindings.of(parseCondition('o instanceof String s || s.length() > 0'))
+ assert b.whenTrue().isEmpty()
+ assert b.whenFalse().isEmpty()
+ assert b.isEmpty()
+ }
+
+ @Test
+ void testBindingsOfNegatedOrFalsePath() {
+ def b = InstanceofFlowBindings.of(parseCondition('!(o instanceof String s) || s.isEmpty()'))
+ // left false-path binds s; OR false bindings = left.whenFalse ∪ right.whenFalse = {s}
+ assert b.whenFalse()*.name == ['s']
+ assert b.whenTrue().isEmpty()
+ }
+
+ @Test
+ void testBindingsOfPlainInstanceofAreEmpty() {
+ def b = InstanceofFlowBindings.of(parseCondition('o instanceof String'))
+ assert b.isEmpty()
+ }
+
+ @Test
+ void testOfNullIsEmpty() {
+ assert InstanceofFlowBindings.of(null).isEmpty()
+ }
+
+ @Test
+ void testContainsPattern() {
+ assert InstanceofFlowBindings.containsPattern(parseCondition('o instanceof String s'))
+ assert InstanceofFlowBindings.containsPattern(parseCondition('!(o instanceof String s)'))
+ assert InstanceofFlowBindings.containsPattern(parseCondition('o instanceof String s && s'))
+ assert !InstanceofFlowBindings.containsPattern(parseCondition('o instanceof String'))
+ assert !InstanceofFlowBindings.containsPattern(null)
+ }
+
+ @Test
+ void testContainsPatternNestedInCall() {
+ def expr = parseMethodArg('m(o instanceof String s)')
+ assert InstanceofFlowBindings.containsPattern(expr)
+ }
+
+ @Test
+ void testRightOfOrIsDynamicVariable() {
+ def src = '''
+ class C {
+ def m(Object o) {
+ if (o instanceof String s || s.length() > 0) {
+ return 1
+ }
+ return 0
+ }
+ }
+ '''
+ def cu = new CompilationUnit()
+ cu.addSource('C.groovy', src)
+ cu.compile(Phases.SEMANTIC_ANALYSIS)
+ def accesses = []
+ cu.ast.classes[0].getMethods('m')[0].code.visit(new CodeVisitorSupport() {
+ @Override
+ void visitVariableExpression(VariableExpression ve) {
+ if (ve.name == 's') {
+ accesses << ve.accessedVariable
+ }
+ super.visitVariableExpression(ve)
+ }
+ })
+ assert accesses.any { it instanceof DynamicVariable } :
+ "expected DynamicVariable for RHS of ||, got: ${accesses*.class*.simpleName}"
+ }
+
+ @Test
+ void testNegatedIfBranchIsDynamicVariable() {
+ def src = '''
+ class C {
+ def m(Object o) {
+ if (!(o instanceof String s)) {
+ return s
+ }
+ return null
+ }
+ }
+ '''
+ def cu = new CompilationUnit()
+ cu.addSource('C.groovy', src)
+ cu.compile(Phases.SEMANTIC_ANALYSIS)
+ def accesses = []
+ cu.ast.classes[0].getMethods('m')[0].code.visit(new CodeVisitorSupport() {
+ @Override
+ void visitVariableExpression(VariableExpression ve) {
+ if (ve.name == 's') {
+ accesses << ve.accessedVariable
+ }
+ super.visitVariableExpression(ve)
+ }
+ })
+ // declaration + use in if-branch (must be DynamicVariable)
+ assert accesses.count { it instanceof DynamicVariable } >= 1
+ }
+
+ private static Expression parseCondition(String condition) {
+ def src = """
+ class C {
+ def m(Object o) {
+ if ($condition) return 1
+ return 0
+ }
+ }
+ """
+ def cu = new CompilationUnit()
+ cu.addSource('C.groovy', src)
+ cu.compile(Phases.CONVERSION)
+ IfStatement ifStmt = null
+ cu.ast.classes[0].getMethods('m')[0].code.visit(new CodeVisitorSupport() {
+ @Override
+ void visitIfElse(IfStatement statement) {
+ ifStmt = statement
+ }
+ })
+ ifStmt.booleanExpression.expression
+ }
+
+ private static Expression parseMethodArg(String statement) {
+ def src = """
+ class C {
+ def m(Object o) {
+ $statement
+ }
+ }
+ """
+ def cu = new CompilationUnit()
+ cu.addSource('C.groovy', src)
+ cu.compile(Phases.CONVERSION)
+ Expression found = null
+ cu.ast.classes[0].getMethods('m')[0].code.visit(new CodeVisitorSupport() {
+ @Override
+ void visitMethodCallExpression(org.codehaus.groovy.ast.expr.MethodCallExpression call) {
+ if (call.methodAsString == 'm' || call.objectExpression.text == 'this') {
+ found = call
+ }
+ super.visitMethodCallExpression(call)
+ }
+ @Override
+ void visitExpressionStatement(org.codehaus.groovy.ast.stmt.ExpressionStatement stmt) {
+ found = stmt.expression
+ super.visitExpressionStatement(stmt)
+ }
+ })
+ found
+ }
+}
diff --git a/src/test/groovy/groovy/InstanceofTest.groovy b/src/test/groovy/groovy/InstanceofTest.groovy
index 7896c65ac9..4caa670509 100644
--- a/src/test/groovy/groovy/InstanceofTest.groovy
+++ b/src/test/groovy/groovy/InstanceofTest.groovy
@@ -223,4 +223,279 @@ final class InstanceofTest {
}
assert y == 'foobar'
}
+
+ // GROOVY-12242: Java-aligned flow scoping for negated instanceof (JEP 394)
+ @Test
+ void testVariableScopeNegatedElse() {
+ def f = { Object o ->
+ if (!(o instanceof String s)) {
+ return 'not'
+ } else {
+ return s.toUpperCase()
+ }
+ }
+ assert f('hi') == 'HI'
+ assert f(1) == 'not'
+ }
+
+ // GROOVY-12242: pattern variable remains in scope after abrupt then-branch
+ @Test
+ void testVariableScopeEarlyReturn() {
+ def f = { Object o ->
+ if (!(o instanceof String s)) return 'early'
+ return s.toUpperCase()
+ }
+ assert f('hi') == 'HI'
+ assert f(42) == 'early'
+ }
+
+ // GROOVY-12242: pattern variable remains after else that cannot complete normally
+ @Test
+ void testVariableScopeAfterAbruptElse() {
+ def f = { Object o ->
+ if (o instanceof String s) {
+ // matched
+ } else {
+ return 'no'
+ }
+ return s.toUpperCase()
+ }
+ assert f('ab') == 'AB'
+ assert f(9) == 'no'
+ }
+
+ // GROOVY-12242: pattern variable must not leak after a declaration statement
+ @Test
+ void testVariableNoLeakAfterDeclaration() {
+ def err = shouldFail MissingPropertyException, '''
+ class C {
+ Object m(Object o) {
+ boolean b = (o instanceof String s)
+ return s
+ }
+ }
+ new C().m('hi')
+ '''
+ assert err.message =~ /No such property: s/
+ }
+
+ // GROOVY-12242: pattern variable must not leak after an expression statement
+ @Test
+ void testVariableNoLeakAfterExpressionStatement() {
+ def err = shouldFail MissingPropertyException, '''
+ class C {
+ Object m(Object o) {
+ o instanceof String s && s.length() > 0
+ return s
+ }
+ }
+ new C().m('hi')
+ '''
+ assert err.message =~ /No such property: s/
+ }
+
+ // GROOVY-12242: true branch of negated instanceof must not see the pattern local
+ // (CompileStack polarity must match VariableScope — no silent null ALOAD)
+ @Test
+ void testVariableNegatedIfBranchNotInScope() {
+ def err = shouldFail MissingPropertyException, '''
+ class C {
+ Object m(Object o) {
+ if (!(o instanceof String s)) {
+ return s
+ }
+ return 'matched'
+ }
+ }
+ new C().m(1)
+ '''
+ assert err.message =~ /No such property: s/
+ }
+
+ // GROOVY-12242: true-path binding of left of || is not in scope on the right (Java)
+ @Test
+ void testVariableOrRightHandSideNotInScope() {
+ def shell = GroovyShell.withConfig {
+ ast groovy.transform.TypeChecked
+ }
+ def err = shouldFail shell, '''
+ @groovy.transform.TypeChecked
+ class C {
+ static void m(Object o) {
+ if (o instanceof String s || s.length() > 0) {
+ }
+ }
+ }
+ '''
+ assert err.message =~ /The variable .s. is undeclared|Apparent variable .s./
+ }
+
+ // GROOVY-12242: false-path binding is in scope on the right of || (Java)
+ @Test
+ void testVariableOrRightHandSideFalsePathInScope() {
+ def f = { Object o ->
+ // when o is String, left is false, right sees s
+ return (!(o instanceof String s) || s.isEmpty())
+ }
+ assert f('') == true
+ assert f('x') == false
+ assert f(1) == true // left true → short-circuit, s not needed
+ }
+
+ // GROOVY-12242: ternary false branch must not see true-path pattern variable
+ @Test
+ void testVariableTernaryFalseBranchNotInScope() {
+ def shell = GroovyShell.withConfig {
+ ast groovy.transform.TypeChecked
+ }
+ def err = shouldFail shell, '''
+ @groovy.transform.TypeChecked
+ class C {
+ static Object m(Object o) {
+ return o instanceof String s ? 'yes' : s
+ }
+ }
+ '''
+ assert err.message =~ /The variable .s. is undeclared|Apparent variable .s./
+ }
+
+ // GROOVY-12242: dynamic ternary false branch must not load a pattern local
+ @Test
+ void testVariableTernaryFalseBranchNotInScopeDynamic() {
+ def err = shouldFail MissingPropertyException, '''
+ class C {
+ Object m(Object o) {
+ return o instanceof String s ? 'yes' : s
+ }
+ }
+ new C().m(1)
+ '''
+ assert err.message =~ /No such property: s/
+ }
+
+ // GROOVY-12242: ternary true branch sees pattern variable
+ @Test
+ void testVariableTernaryTrueBranch() {
+ def f = { Object o -> o instanceof String s ? s.toUpperCase() : 'no' }
+ assert f('ab') == 'AB'
+ assert f(1) == 'no'
+ }
+
+ // GROOVY-12242: reassignment of pattern variable (not implicitly final, JEP 394)
+ @Test
+ void testVariableReassignment() {
+ Object o = 'hi'
+ if (o instanceof String s) {
+ s = s + '!'
+ assert s == 'hi!'
+ } else {
+ assert false
+ }
+ }
+
+ // GROOVY-12242: pattern variable shadows a field only where in scope
+ @Test
+ void testVariableFieldShadowing() {
+ def obj = new Object() {
+ String s = 'field'
+ def test(Object o) {
+ if (o instanceof String s) {
+ return "pv=$s"
+ }
+ return "field=$s"
+ }
+ }
+ assert obj.test('x') == 'pv=x'
+ assert obj.test(1) == 'field=field'
+ }
+
+ // GROOVY-12242: && chain uses pattern variable on subsequent operands
+ @Test
+ void testVariableAndChain() {
+ Object o = 'hello'
+ assert (o instanceof String s && s.length() > 3 && s.startsWith('h'))
+ assert !(o instanceof String s && s.length() > 99)
+ }
+
+ // GROOVY-12242: while body can use true-path pattern variable
+ @Test
+ void testVariableWhileBody() {
+ Object o = 'ab'
+ def n = 0
+ while (o instanceof String s && s.length() > 0) {
+ n += 1
+ o = s.substring(1)
+ }
+ assert n == 2
+ assert o == ''
+ }
+
+ // GROOVY-12242: reuse the same pattern variable name in successive statements
+ @Test
+ void testVariableNameReuse() {
+ Object a = 'x', b = 1
+ def r = []
+ if (a instanceof String s) r << s
+ if (b instanceof Integer s) r << s
+ assert r == ['x', 1]
+ }
+
+ // GROOVY-12242: type-checked flow scoping for early return
+ @Test
+ void testVariableScopeEarlyReturnTypeChecked() {
+ def shell = GroovyShell.withConfig {
+ ast groovy.transform.TypeChecked
+ }
+ assert shell.evaluate('''
+ @groovy.transform.TypeChecked
+ class C {
+ static String m(Object o) {
+ if (!(o instanceof String s)) return 'early'
+ return s.toUpperCase()
+ }
+ }
+ assert C.m('hi') == 'HI'
+ assert C.m(1) == 'early'
+ true
+ ''')
+ }
+
+ // GROOVY-12242: type-checked — positive instanceof still not in else
+ @Test
+ void testVariableScopePositiveNotInElseTypeChecked() {
+ def shell = GroovyShell.withConfig {
+ ast groovy.transform.TypeChecked
+ }
+ def err = shouldFail shell, '''
+ Number n = 12345
+ if (n instanceof Integer i) {
+ } else {
+ i.toString()
+ }
+ '''
+ assert err.message =~ /The variable .i. is undeclared/
+ }
+
+ // GROOVY-12242: type-checked — negated instanceof is in else
+ @Test
+ void testVariableScopeNegatedInElseTypeChecked() {
+ def shell = GroovyShell.withConfig {
+ ast groovy.transform.TypeChecked
+ }
+ assert shell.evaluate('''
+ @groovy.transform.TypeChecked
+ class C {
+ static String m(Object o) {
+ if (!(o instanceof String s)) {
+ return 'not'
+ } else {
+ return s.toUpperCase()
+ }
+ }
+ }
+ assert C.m('hi') == 'HI'
+ assert C.m(1) == 'not'
+ true
+ ''')
+ }
}