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:37 UTC
(groovy) 07/07: GROOVY-12242: emit one LocalVariableTable entry per pattern-variable visibility range
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 fbf16d5f1995bfd2ba541e537f743454e8e9cd2f
Author: Paul King <pa...@asert.com.au>
AuthorDate: Wed Aug 12 10:21:28 2026 +1000
GROOVY-12242: emit one LocalVariableTable entry per pattern-variable visibility range
Flow scoping hides pattern slots on paths where the name is not live,
but each local got a single contiguous LVT range, so debug info claimed
visibility over dead regions (e.g. the else-arm of the binding if) and
a redeclared same-named local overlapped the pattern entry. Hiding now
closes the open range at the hide point, and a pop that re-exposes the
variable opens a new one; clear() emits one entry per non-zero-width
range, the same shape javac emits for Java pattern variables. Ordinary
locals never close a range and are emitted exactly as before.
---
.../groovy/classgen/asm/BytecodeVariable.java | 63 +++++++
.../codehaus/groovy/classgen/asm/CompileStack.java | 84 ++++++++-
.../classgen/asm/InstanceofPatternLvtTest.groovy | 192 +++++++++++++++++++++
3 files changed, 334 insertions(+), 5 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/BytecodeVariable.java b/src/main/java/org/codehaus/groovy/classgen/asm/BytecodeVariable.java
index 0c8850b074..ed11bbc646 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/BytecodeVariable.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/BytecodeVariable.java
@@ -22,6 +22,10 @@ import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.objectweb.asm.Label;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
/**
* Represents compile time variable metadata while compiling a method.
*/
@@ -48,6 +52,15 @@ public class BytecodeVariable {
private Label startLabel;
private Label endLabel;
+ // GROOVY-12242: flow-scoped pattern variables are visible over several
+ // disjoint bytecode regions (e.g. condition + then-arm, then again after
+ // the if). Each completed region is kept here and emitted as its own
+ // LocalVariableTable entry; startLabel/endLabel describe the currently
+ // open region only. Ordinary locals never close a range, so this stays
+ // null for them and emission is unchanged.
+ private List<Label[]> closedRanges;
+ private boolean rangeClosed;
+
private BytecodeVariable() {
index = 0;
prevCurrent = 0;
@@ -172,6 +185,56 @@ public class BytecodeVariable {
this.endLabel = endLabel;
}
+ /**
+ * Ends the currently open local-variable-table range at the given label,
+ * recording it for later emission. Used when flow scoping hides this
+ * variable (GROOVY-12242); a later {@link #reopenRange} starts a new range
+ * if the variable becomes visible again.
+ *
+ * @param end the label at which visibility ends; ignored if {@code null}
+ * or if no range is open
+ */
+ public void closeRange(final Label end) {
+ if (!rangeClosed && startLabel != null && end != null) {
+ if (closedRanges == null) closedRanges = new ArrayList<>(2);
+ closedRanges.add(new Label[]{startLabel, end});
+ rangeClosed = true;
+ }
+ }
+
+ /**
+ * Starts a new local-variable-table range at the given label after a
+ * {@link #closeRange}. The end label is reset to the start and extended
+ * by subsequent scope pops; a range that is never extended has zero
+ * width and is not emitted.
+ *
+ * @param start the label at which visibility resumes; ignored if
+ * {@code null} or if no range was closed
+ */
+ public void reopenRange(final Label start) {
+ if (rangeClosed && start != null) {
+ startLabel = start;
+ endLabel = start;
+ rangeClosed = false;
+ }
+ }
+
+ /**
+ * @return whether the current local-variable-table range has been closed
+ * without being reopened (the variable is hidden by flow scoping)
+ */
+ public boolean isRangeClosed() {
+ return rangeClosed;
+ }
+
+ /**
+ * @return the completed local-variable-table ranges as {start, end} label
+ * pairs, oldest first; empty for variables never hidden
+ */
+ public List<Label[]> getClosedRanges() {
+ return closedRanges == null ? Collections.emptyList() : closedRanges;
+ }
+
/**
* {@inheritDoc}
*/
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 c708a287c0..8d7a5b74c7 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/CompileStack.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/CompileStack.java
@@ -32,6 +32,7 @@ import org.objectweb.asm.MethodVisitor;
import org.objectweb.asm.TypePath;
import org.objectweb.asm.TypeReference;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Deque;
@@ -296,6 +297,26 @@ public class CompileStack {
stackVariables = element.stackVariables;
finallyBlocks = element.finallyBlocks;
inSpecialConstructorCall = element.inSpecialConstructorCall;
+ if (!clear) reopenHiddenVariableRanges();
+ }
+
+ /**
+ * Opens a new LocalVariableTable range for any variable in the restored
+ * frame whose range was closed by {@link #hideVariable} in the frame just
+ * popped — the pop makes the name visible again, so debug info must resume
+ * here. Skipped during {@link #clear} (code emission is already complete).
+ */
+ private void reopenHiddenVariableRanges() {
+ Label here = null;
+ for (BytecodeVariable variable : stackVariables.values()) {
+ if (variable.isRangeClosed()) {
+ if (here == null) {
+ here = new Label();
+ controller.getMethodVisitor().visitLabel(here);
+ }
+ variable.reopenRange(here);
+ }
+ }
}
/**
@@ -451,9 +472,19 @@ public class CompileStack {
}
for (BytecodeVariable v : usedVariables) {
String type = BytecodeHelper.getTypeDescription(v.isHolder() ? ClassHelper.REFERENCE_TYPE : v.getType());
- Label startLabel = v.getStartLabel(), endLabel = v.getEndLabel();
- if (endLabel == null) endLabel = startLabel; // only occurs for '_' placeholder
- mv.visitLocalVariable(v.getName(), type, null, startLabel, endLabel, v.getIndex());
+ // GROOVY-12242: one table entry per visibility range (several for
+ // flow-scoped pattern variables; exactly one for ordinary locals)
+ List<Label[]> ranges = localVariableRanges(v);
+ if (ranges.isEmpty()) continue;
+ int n = ranges.size();
+ Label[] startLabels = new Label[n], endLabels = new Label[n];
+ int[] indices = new int[n];
+ for (int i = 0; i < n; i += 1) {
+ startLabels[i] = ranges.get(i)[0];
+ endLabels[i] = ranges.get(i)[1];
+ indices[i] = v.getIndex();
+ mv.visitLocalVariable(v.getName(), type, null, startLabels[i], endLabels[i], v.getIndex());
+ }
// JSR 308: local variable type annotations
ClassNode t = v.getType();
String typePath = ""; // ?
@@ -461,7 +492,7 @@ public class CompileStack {
for (AnnotationNode a : t.getTypeAnnotations()) {
type = BytecodeHelper.getTypeDescription(a.getClassNode());
var av = mv.visitLocalVariableAnnotation(TypeReference.LOCAL_VARIABLE << 24, TypePath.fromString(typePath),
- new Label[]{startLabel}, new Label[]{endLabel}, new int[]{v.getIndex()}, type, a.hasRuntimeRetention());
+ startLabels, endLabels, indices, type, a.hasRuntimeRetention());
if (av != null) {
controller.getAcg().visitAnnotationAttributes(a, av);
av.visitEnd();
@@ -501,6 +532,38 @@ public class CompileStack {
scope = null;
}
+ /**
+ * The LocalVariableTable ranges to emit for a variable: each closed range
+ * (GROOVY-12242 flow-scoped hides) plus the still-open range, zero-width
+ * ranges omitted. A variable that was never hidden keeps today's single
+ * entry, including the zero-width form used for the {@code '_'} placeholder.
+ */
+ private static List<Label[]> localVariableRanges(final BytecodeVariable v) {
+ List<Label[]> closed = v.getClosedRanges();
+ if (closed.isEmpty()) {
+ Label startLabel = v.getStartLabel(), endLabel = v.getEndLabel();
+ if (endLabel == null) endLabel = startLabel; // only occurs for '_' placeholder
+ return Collections.singletonList(new Label[]{startLabel, endLabel});
+ }
+ List<Label[]> ranges = new ArrayList<>(closed.size() + 1);
+ for (Label[] range : closed) {
+ if (hasWidth(range[0], range[1])) ranges.add(range);
+ }
+ if (!v.isRangeClosed() && hasWidth(v.getStartLabel(), v.getEndLabel())) {
+ ranges.add(new Label[]{v.getStartLabel(), v.getEndLabel()});
+ }
+ return ranges;
+ }
+
+ private static boolean hasWidth(final Label start, final Label end) {
+ if (start == null || end == null) return false;
+ try {
+ return start.getOffset() != end.getOffset();
+ } catch (IllegalStateException e) {
+ return true; // offsets not resolved (non-writer visitor); keep the range
+ }
+ }
+
/**
* Adds a typed or untyped exception handler to the exception table for
* the current method. Typed entries (non-null {@code sig}) are emitted
@@ -957,10 +1020,21 @@ public class CompileStack {
* still be needed on another path that re-exposes the same slot via pop or a
* later path that still lists the name as live.
*
+ * <p>
+ * The variable's LocalVariableTable range is closed at the hide point so
+ * debug info never claims visibility over a region where flow scoping has
+ * removed the name; if a later {@link #pop} re-exposes the variable, a new
+ * range is opened (emitted as an additional table entry by {@link #clear}).
+ *
* @param name the variable name to hide; no-op if absent
*/
public void hideVariable(final String name) {
- stackVariables.remove(name);
+ BytecodeVariable variable = stackVariables.remove(name);
+ if (variable != null && !variable.isRangeClosed()) {
+ Label here = new Label();
+ controller.getMethodVisitor().visitLabel(here);
+ variable.closeRange(here);
+ }
}
/**
diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/InstanceofPatternLvtTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/InstanceofPatternLvtTest.groovy
new file mode 100644
index 0000000000..70cfe42d1b
--- /dev/null
+++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/InstanceofPatternLvtTest.groovy
@@ -0,0 +1,192 @@
+/*
+ * 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.junit.jupiter.api.Test
+import org.objectweb.asm.ClassReader
+import org.objectweb.asm.tree.ClassNode
+import org.objectweb.asm.tree.LdcInsnNode
+import org.objectweb.asm.tree.MethodNode
+
+/**
+ * GROOVY-12242: checks that the LocalVariableTable debug ranges emitted for
+ * {@code instanceof} pattern variables agree with flow scoping. A pattern
+ * variable may be visible over several disjoint bytecode regions (e.g. the
+ * condition plus the then-arm, then again after the if), so entries are
+ * checked by coverage properties rather than by count:
+ * <ul>
+ * <li>no entry for the pattern name covers a region where flow scoping
+ * says the name is dead (negative), and</li>
+ * <li>every region where the name is live is covered by some entry
+ * (positive), so debuggers keep showing the variable there.</li>
+ * </ul>
+ */
+final class InstanceofPatternLvtTest extends AbstractBytecodeTestCase {
+
+ private MethodNode treeMethod(final String name) {
+ assert classBytes != null : 'no class bytes captured'
+ def cn = new ClassNode()
+ new ClassReader(classBytes).accept(cn, 0)
+ def mn = cn.methods.find { it.name == name }
+ assert mn != null : "method '$name' not found; methods: ${cn.methods*.name}"
+ mn
+ }
+
+ /** LVT entries for {@code name} as maps of desc/slot/start/end (instruction indices). */
+ private static List<Map> entriesFor(final MethodNode mn, final String name) {
+ (mn.localVariables ?: []).findAll { it.name == name }.collect { lv ->
+ [desc: lv.desc, slot: lv.index,
+ start: mn.instructions.indexOf(lv.start),
+ end: mn.instructions.indexOf(lv.end)]
+ }
+ }
+
+ private static boolean covers(final Map entry, final int idx) {
+ entry.start <= idx && idx < entry.end
+ }
+
+ private static int ldcIndex(final MethodNode mn, final Object cst) {
+ for (int i = 0; i < mn.instructions.size(); i += 1) {
+ def insn = mn.instructions.get(i)
+ if (insn instanceof LdcInsnNode && insn.cst == cst) return i
+ }
+ -1
+ }
+
+ private static int marker(final MethodNode mn, final String cst) {
+ int idx = ldcIndex(mn, cst)
+ assert idx != -1 : "marker '$cst' not found"
+ idx
+ }
+
+ private static String describe(final MethodNode mn) {
+ (mn.localVariables ?: []).collect { lv ->
+ "LVT ${lv.name} ${lv.desc} slot=${lv.index} range=[${mn.instructions.indexOf(lv.start)}, ${mn.instructions.indexOf(lv.end)})"
+ }.join('\n')
+ }
+
+ private void assertNoneCover(final MethodNode mn, final List<Map> entries, final String markerText) {
+ int idx = marker(mn, markerText)
+ entries.each { e ->
+ assert !covers(e, idx) :
+ "entry ${e} covers dead-region marker '$markerText' at $idx\n${describe(mn)}"
+ }
+ }
+
+ private void assertSomeCover(final MethodNode mn, final List<Map> entries, final String markerText) {
+ int idx = marker(mn, markerText)
+ assert entries.any { covers(it, idx) } :
+ "no entry covers live-region marker '$markerText' at $idx; entries: ${entries}\n${describe(mn)}"
+ }
+
+ // case 1: pattern variable dead in the else-arm, live in the then-arm
+ @Test
+ void testPatternVariableRangeExcludesElseArm() {
+ compile method: 'm', '''
+ def m(o) {
+ if (o instanceof String s) {
+ print 'in-then'
+ print s
+ } else {
+ print 'in-else'
+ }
+ }
+ m('x')
+ '''
+ def mn = treeMethod('m')
+ def entries = entriesFor(mn, 's')
+ assert !entries.empty : "no LVT entries for s\n${describe(mn)}"
+ assertNoneCover(mn, entries, 'in-else')
+ assertSomeCover(mn, entries, 'in-then')
+ }
+
+ // case 2: redeclared local in else must not overlap the pattern variable's entries
+ @Test
+ void testRedeclaredLocalRangeDoesNotOverlapPattern() {
+ compile method: 'm', '''
+ def m(o) {
+ if (o instanceof String s) {
+ print 'in-then'
+ print s
+ } else {
+ def s = 'local'
+ print 'else-after-decl'
+ print s
+ }
+ }
+ m('x')
+ '''
+ def mn = treeMethod('m')
+ def entries = entriesFor(mn, 's')
+ def pattern = entries.findAll { it.desc == 'Ljava/lang/String;' }
+ def redeclared = entries.findAll { it.desc != 'Ljava/lang/String;' }
+ assert !pattern.empty && !redeclared.empty : "expected both s variables in LVT\n${describe(mn)}"
+ assert pattern*.slot.unique() != redeclared*.slot.unique() : 'expected distinct slots'
+ pattern.each { p ->
+ redeclared.each { r ->
+ boolean overlap = p.start < r.end && r.start < p.end
+ assert !overlap : "pattern entry ${p} overlaps redeclared entry ${r}\n${describe(mn)}"
+ }
+ }
+ assertNoneCover(mn, pattern, 'else-after-decl')
+ assertSomeCover(mn, redeclared, 'else-after-decl')
+ assertSomeCover(mn, pattern, 'in-then')
+ }
+
+ // case 3: negated pattern with abrupt then — dead in the then-arm, live after the if
+ @Test
+ void testNegatedPatternRangeExcludesThenArm() {
+ compile method: 'm', '''
+ def m(o) {
+ if (!(o instanceof String s)) {
+ print 'in-then'
+ return
+ }
+ print 'after-if'
+ print s
+ }
+ m('x')
+ '''
+ def mn = treeMethod('m')
+ def entries = entriesFor(mn, 's')
+ assert !entries.empty : "no LVT entries for s\n${describe(mn)}"
+ assertNoneCover(mn, entries, 'in-then')
+ assertSomeCover(mn, entries, 'after-if')
+ }
+
+ // case 4: no-binding shape (`instanceof s || cond`) — dead in both arms
+ @Test
+ void testOrShapePatternRangeCoversNeitherArm() {
+ compile method: 'm', '''
+ def m(o, c) {
+ if (o instanceof String s || c) {
+ print 'in-then'
+ } else {
+ print 'in-else'
+ }
+ }
+ m('x', true)
+ '''
+ def mn = treeMethod('m')
+ def entries = entriesFor(mn, 's')
+ assert !entries.empty : "no LVT entries for s\n${describe(mn)}"
+ assertNoneCover(mn, entries, 'in-then')
+ assertNoneCover(mn, entries, 'in-else')
+ }
+}