This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new f2d3aace3b GROOVY-11229: limit scope of `instanceof` variable
f2d3aace3b is described below
commit f2d3aace3b3ed9735336e30937ea18e221a9ca71
Author: Eric Milles <[email protected]>
AuthorDate: Sat Apr 5 09:40:12 2025 -0500
GROOVY-11229: limit scope of `instanceof` variable
---
.../codehaus/groovy/classgen/asm/CompileStack.java | 7 ++--
.../groovy/classgen/asm/StatementWriter.java | 38 +++++++++++++---------
src/test/groovy/InstanceofTest.groovy | 11 ++++++-
3 files changed, 37 insertions(+), 19 deletions(-)
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 f4eea5c081..3939be0bc4 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/CompileStack.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/CompileStack.java
@@ -576,9 +576,12 @@ public class CompileStack {
}
/**
- * because a boolean Expression may not be evaluated completely
- * it is important to keep the registers clean
+ * Because a boolean Expression may not be evaluated completely
+ * it is important to keep the registers clean.
+ *
+ * @deprecated Use {@link #pushState()} directly.
*/
+ @Deprecated(since = "5.0.0")
public void pushBooleanExpression() {
pushState();
}
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 3a06162e0c..bd7387661a 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
@@ -306,9 +306,14 @@ public class StatementWriter {
controller.getAcg().onLineNumber(statement, "visitIfElse");
writeStatementLabel(statement);
+ CompileStack compileStack = controller.getCompileStack();
+ compileStack.pushState();
+
statement.getBooleanExpression().visit(controller.getAcg());
Label elsePath = controller.getOperandStack().jump(IFEQ);
statement.getIfBlock().visit(controller.getAcg());
+ compileStack.pop();
+
MethodVisitor mv = controller.getMethodVisitor();
if (statement.getElseBlock().isEmpty()) {
mv.visitLabel(elsePath);
@@ -436,10 +441,11 @@ public class StatementWriter {
statement.getExpression().visit(controller.getAcg());
- // switch does not have a continue label. use its parent's for continue
- Label breakLabel = controller.getCompileStack().pushSwitch();
+ // switch does not have a continue label; use enclosing continue label
+ CompileStack compileStack = controller.getCompileStack();
+ Label breakLabel = compileStack.pushSwitch();
- int switchVariableIndex =
controller.getCompileStack().defineTemporaryVariable("switch", true);
+ int switchVariableIndex =
compileStack.defineTemporaryVariable("switch", true);
List<CaseStatement> caseStatements = statement.getCaseStatements();
int caseCount = caseStatements.size();
@@ -456,9 +462,8 @@ public class StatementWriter {
statement.getDefaultStatement().visit(controller.getAcg());
controller.getMethodVisitor().visitLabel(breakLabel);
-
- controller.getCompileStack().removeVar(switchVariableIndex);
- controller.getCompileStack().pop();
+ compileStack.removeVar(switchVariableIndex);
+ compileStack.pop();
}
private void writeCaseStatement(final CaseStatement statement, final int
switchVariableIndex, final Label thisLabel, final Label nextLabel) {
@@ -576,12 +581,13 @@ public class StatementWriter {
public void writeReturn(final ReturnStatement statement) {
controller.getAcg().onLineNumber(statement, "visitReturnStatement");
writeStatementLabel(statement);
- ClassNode rType = controller.getReturnType();
- CompileStack cs = controller.getCompileStack();
- OperandStack os = controller.getOperandStack();
- MethodVisitor mv = controller.getMethodVisitor();
- if (ClassHelper.isPrimitiveVoid(rType)) {
+ var cs = controller.getCompileStack();
+ var os = controller.getOperandStack();
+ var mv = controller.getMethodVisitor();
+
+ var returnType = controller.getReturnType();
+ if (ClassHelper.isPrimitiveVoid(returnType)) {
if (!statement.isReturningNullOrVoid()) { // TODO: move to Verifier
controller.getAcg().throwException("Cannot use return
statement with an expression on a method that returns void");
}
@@ -591,21 +597,21 @@ public class StatementWriter {
Expression expression = statement.getExpression();
expression.visit(controller.getAcg());
- if (!isNullConstant(expression) ||
ClassHelper.isPrimitiveType(rType)) {
- os.doGroovyCast(rType);
+ if (!isNullConstant(expression) ||
ClassHelper.isPrimitiveType(returnType)) {
+ os.doGroovyCast(returnType);
} else { // GROOVY-10617
- os.replace(rType);
+ os.replace(returnType);
}
if (cs.hasBlockRecorder()) {
ClassNode top = os.getTopOperand();
- int returnVal = cs.defineTemporaryVariable("returnValue",
rType, true);
+ int returnVal = cs.defineTemporaryVariable("returnValue",
returnType, true);
cs.applyBlockRecorder();
os.load(top, returnVal);
cs.removeVar(returnVal);
}
- BytecodeHelper.doReturn(mv, rType);
+ BytecodeHelper.doReturn(mv, returnType);
os.remove(1);
}
}
diff --git a/src/test/groovy/InstanceofTest.groovy
b/src/test/groovy/InstanceofTest.groovy
index f3c75f83af..01b4349791 100644
--- a/src/test/groovy/InstanceofTest.groovy
+++ b/src/test/groovy/InstanceofTest.groovy
@@ -109,11 +109,20 @@ final class InstanceofTest {
// GROOVY-11229
@Test
void testVariableScope() {
+ def err = shouldFail '''
+ def x = null
+ if (x instanceof String s) {
+ } else {
+ s
+ }
+ '''
+ assert err =~ /No such property: s/
+
def shell = GroovyShell.withConfig {
ast groovy.transform.TypeChecked
}
- def err = shouldFail shell, '''
+ err = shouldFail shell, '''
Number n = 12345
if (n instanceof Integer i) {
}