This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch GROOVY-12162
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY-12162 by this push:
new 874133258e Minor tweak
874133258e is described below
commit 874133258e1175b9e831bd1474d5f139025cf1fb
Author: Daniel Sun <[email protected]>
AuthorDate: Fri Jul 17 23:47:19 2026 +0900
Minor tweak
---
.../asm/PeepholeOptimizingMethodVisitor.java | 292 +++++++++++++++++----
src/test/groovy/bugs/Groovy12162.groovy | 80 ++++++
.../asm/PeepholeOptimizingMethodVisitorTest.groovy | 231 +++++++++++++---
3 files changed, 513 insertions(+), 90 deletions(-)
diff --git
a/src/main/java/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitor.java
b/src/main/java/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitor.java
index c152ed8682..425ad72e10 100644
---
a/src/main/java/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitor.java
+++
b/src/main/java/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitor.java
@@ -115,27 +115,37 @@ import static org.objectweb.asm.Opcodes.SIPUSH;
* Signed floating-point zeros ({@code -0.0f}/{@code -0.0d}) are
preserved
* via raw-bit comparison (GROOVY-9797).</li>
* <li><b>Dead loads</b> — a buffered load or constant followed by a matching
- * {@code POP}/{@code POP2}, or a void {@code RETURN}, is dropped. A
- * buffered {@code IINC} paired with {@code ILOAD} is retained as a side
- * effect when the loaded value itself is discarded.</li>
+ * {@code POP}/{@code POP2}, or a void {@code RETURN}, is dropped when
the
+ * load is pure (no attached {@code CHECKCAST}). A buffered {@code IINC}
+ * paired with {@code ILOAD} is retained as a side effect when the loaded
+ * value itself is discarded.</li>
* <li><b>CHECKCAST</b> — may attach to a pending {@code ALOAD} or reference
- * constant so {@code load}; {@code CHECKCAST}; {@code POP} collapses
- * entirely. A standalone cast before {@code POP} is dropped; before void
- * {@code RETURN} the cast is flushed and the value is popped so the
- * type-check side effect is kept and the void return stays
verifiable.</li>
+ * constant so the cast is emitted immediately after the load on flush.
+ * When the cast result is discarded ({@code POP} or void {@code
RETURN}),
+ * the cast is <em>always</em> kept so a possible {@code
ClassCastException}
+ * remains observable — matching Java and the void-return path. Pure
loads
+ * without a cast still collapse with the pop. Standalone casts of a
value
+ * already on the stack are likewise preserved before pop/return, with
+ * {@code POP} inserted before void {@code RETURN} so the frame stays
+ * verifiable.</li>
* <li><b>Compare-to-zero / null</b> —
* {@code ICONST_0}; {@code IF_ICMP*} → {@code IF*};
* {@code ACONST_NULL}; {@code IF_ACMP*} → {@code IFNULL}/{@code
IFNONNULL}.</li>
* <li><b>DUP + store + pop</b> — {@code DUP}/{@code DUP2}; store;
* matching pop becomes a plain store; bare {@code DUP}/{@code DUP2}
with a
* matching pop is eliminated.</li>
- * <li><b>Box/unbox cancellation</b> — {@code Wrapper.valueOf(p)};
- * {@code Wrapper.xxxValue()} and {@code
DefaultTypeTransformation.box(p)};
- * {@code DefaultTypeTransformation.xxxUnbox(...)} with matching
primitive
- * types cancel to a no-op (the primitive remains on the stack). A boxed
- * value discarded by {@code POP}/{@code POP2} drops the box and pops the
- * original primitive instead (using {@code POP2} when the primitive is
- * wide). Mismatched unbox types are left intact.</li>
+ * <li><b>Box/unbox cancellation</b> — {@code Wrapper.valueOf(p)} /
+ * {@code DefaultTypeTransformation.box(p)} followed by a same-type unbox
+ * ({@code Wrapper.xxxValue()} or {@code
DefaultTypeTransformation.xxxUnbox})
+ * cancel to a no-op (the primitive remains on the stack).
Cross-convention
+ * pairs of the same primitive type are included (for example
+ * {@code DTT.box(I)} then {@code Integer.intValue()}). A boxed value
+ * discarded by {@code POP}/{@code POP2} (or void {@code RETURN}) drops
the
+ * box and, when the primitive producer is still in the load window,
drops
+ * that producer too; otherwise it pops the original primitive
+ * ({@code POP2} when wide). {@code POP2} on a narrow boxed value is
treated
+ * as two one-slot discards (box/primitive plus the slot underneath).
+ * Mismatched unbox types are left intact.</li>
* <li><b>Boolean constant folding</b> — {@code GETSTATIC Boolean.TRUE/FALSE}
* followed by {@code booleanValue()} or
* {@code DefaultTypeTransformation.booleanUnbox} becomes
@@ -211,9 +221,11 @@ public final class PeepholeOptimizingMethodVisitor extends
MethodVisitor {
* <ul>
* <li>When {@link #pendingLoadKind} is {@link PendingLoadKind#VARIABLE}
or
* {@link PendingLoadKind#CONSTANT}: optional cast attached to that
load
- * (emitted after the load on flush, or dropped with the load on
pop).</li>
+ * (emitted after the load on flush; retained with the load when the
+ * value is discarded so the type-check side effect is
preserved).</li>
* <li>When the kind is {@link PendingLoadKind#CHECKCAST}: standalone
cast of
- * a value already written to the operand stack.</li>
+ * a value already written to the operand stack (also retained on
+ * discard for the same reason).</li>
* </ul>
*/
private String pendingCheckcastDescriptor;
@@ -620,11 +632,16 @@ public final class PeepholeOptimizingMethodVisitor
extends MethodVisitor {
/**
* Emits a buffered {@code Wrapper.valueOf} or
* {@code DefaultTypeTransformation.box} call to the delegate.
+ * <p>
+ * The primitive operand may still be held in the load window (so that
+ * {@code load}; {@code box}; {@code POP} can collapse entirely). Flush
that
+ * load first so the box always sees its argument on the operand stack.
*/
private void flushPendingBox() {
if (pendingBoxKind == PendingBoxKind.NONE) {
return;
}
+ flushPendingLoad();
switch (pendingBoxKind) {
case VALUE_OF:
super.visitMethodInsn(INVOKESTATIC, pendingBoxOwner, "valueOf",
@@ -718,18 +735,21 @@ public final class PeepholeOptimizingMethodVisitor
extends MethodVisitor {
}
/**
- * Drops a dead buffered load/constant (or standalone {@code CHECKCAST})
when
- * the next instruction is a matching {@code POP}/{@code POP2}.
+ * Handles a matching {@code POP}/{@code POP2} against the load window.
* <p>
- * For a standalone cast the value is already on the stack, so only the
cast is
- * removed and the pop is still emitted. For a buffered load, both the
load and
- * any attached cast are removed; a paired {@code IINC} is preserved as a
side
- * effect.
+ * Pure dead loads/constants are dropped (paired {@code IINC} kept). A
+ * {@code CHECKCAST} — whether attached to the load or standalone — is
+ * <em>not</em> dead: it is emitted so a possible {@code
ClassCastException}
+ * stays observable, and the pop still discards the value. This matches the
+ * void-{@code RETURN} path ({@link #tryDropPendingLoadOnReturn}) and
Java's
+ * treatment of discarded cast expressions.
*
* @return {@code true} if the pop was fully handled
*/
private boolean tryRemovePendingLoad(final int opcode) {
- if (pendingLoadKind == PendingLoadKind.NONE) {
+ // A pending box sits on top of any buffered load; only the box path
may
+ // discard in that case (see tryDropPendingBoxOnPop).
+ if (pendingLoadKind == PendingLoadKind.NONE || pendingBoxKind !=
PendingBoxKind.NONE) {
return false;
}
@@ -739,15 +759,27 @@ public final class PeepholeOptimizingMethodVisitor
extends MethodVisitor {
}
if (pendingLoadKind == PendingLoadKind.CHECKCAST) {
- // Value already on stack: drop the dead cast, keep the pop.
+ // Value already on stack: keep the type-check, keep the pop.
if (opcode != POP) {
return false;
}
+ super.visitTypeInsn(CHECKCAST, pendingCheckcastDescriptor);
clearPendingLoad();
super.visitInsn(POP);
return true;
}
+ // Attached CHECKCAST: same side-effect rule as the void-RETURN path.
+ if (pendingCheckcastDescriptor != null) {
+ if (opcode != POP) {
+ // Reference values occupy one slot; only POP discards them
cleanly.
+ return false;
+ }
+ flushPendingLoad();
+ super.visitInsn(POP);
+ return true;
+ }
+
if (stackSizeForPendingLoad() != popSize) {
return false;
}
@@ -758,8 +790,10 @@ public final class PeepholeOptimizingMethodVisitor extends
MethodVisitor {
}
/**
- * Handles void {@code RETURN} against the load window:
+ * Handles void {@code RETURN} against the load and box windows:
* <ul>
+ * <li>Pending box — the box is dropped; a still-buffered pure primitive
+ * producer is dropped too, otherwise the primitive is popped.</li>
* <li>Pure dead load/constant — dropped (paired {@code IINC} kept).</li>
* <li>Load with attached {@code CHECKCAST}, or standalone cast — the
cast is
* emitted so a type-check side effect is preserved, then {@code
POP} clears
@@ -769,7 +803,17 @@ public final class PeepholeOptimizingMethodVisitor extends
MethodVisitor {
* @return {@code true} if the return was fully handled
*/
private boolean tryDropPendingLoadOnReturn(final int opcode) {
- if (opcode != RETURN || pendingLoadKind == PendingLoadKind.NONE) {
+ if (opcode != RETURN) {
+ return false;
+ }
+
+ if (pendingBoxKind != PendingBoxKind.NONE) {
+ dropPendingBoxDiscardingValue();
+ super.visitInsn(RETURN);
+ return true;
+ }
+
+ if (pendingLoadKind == PendingLoadKind.NONE) {
return false;
}
@@ -1013,8 +1057,14 @@ public final class PeepholeOptimizingMethodVisitor
extends MethodVisitor {
/**
* {@code valueOf}/{@code box} followed by {@code POP}/{@code POP2} means
the
- * boxed reference is discarded. Drop the box and pop the original
primitive
- * instead ({@code POP2} when the primitive is {@code long} or {@code
double}).
+ * boxed reference is discarded. Drop the box; if the primitive producer is
+ * still held in the load window, drop that too, otherwise pop the original
+ * primitive ({@code POP2} when wide).
+ * <p>
+ * {@code POP} after boxing a wide primitive is legal (the boxed reference
is
+ * one slot) and is rewritten to {@code POP2} of the long/double.
+ * {@code POP2} after boxing a <em>narrow</em> primitive is treated as two
+ * one-slot discards: the boxed value plus the slot underneath.
*
* @return {@code true} if the pop was fully handled
*/
@@ -1027,17 +1077,66 @@ public final class PeepholeOptimizingMethodVisitor
extends MethodVisitor {
return false;
}
int primitiveSize =
stackSizeForPrimitiveDescriptor(pendingBoxPrimitiveDescriptor);
- // POP after boxing a wide primitive is legal (boxed ref is one slot)
→ POP2 the long/double.
- if (popSize == 1 && primitiveSize == 2) {
+
+ // Discard only the boxed value. POP on a wide primitive uses POP2 for
the
+ // long/double that remains after the box is dropped.
+ if (popSize == 1 || (popSize == 2 && primitiveSize == 2)) {
+ int slotsToRemove = (popSize == 1 && primitiveSize == 2) ? 2 :
primitiveSize;
clearPendingBox();
- super.visitInsn(POP2);
+ if (tryCancelPendingPrimitiveProducer(slotsToRemove)) {
+ return true;
+ }
+ super.visitInsn(slotsToRemove == 2 ? POP2 : POP);
return true;
}
- if (popSize != primitiveSize) {
- return false;
+
+ // POP2 on a narrow boxed value: two one-slot discards (box +
underneath).
+ if (popSize == 2 && primitiveSize == 1) {
+ clearPendingBox();
+ if (tryCancelPendingPrimitiveProducer(1)) {
+ super.visitInsn(POP);
+ } else {
+ super.visitInsn(POP2);
+ }
+ return true;
}
+
+ return false;
+ }
+
+ /**
+ * Drops a pending box whose result is unused at a void {@code RETURN},
+ * cancelling a still-buffered pure primitive producer when possible.
+ */
+ private void dropPendingBoxDiscardingValue() {
+ int primitiveSize =
stackSizeForPrimitiveDescriptor(pendingBoxPrimitiveDescriptor);
clearPendingBox();
- super.visitInsn(primitiveSize == 2 ? POP2 : POP);
+ if (!tryCancelPendingPrimitiveProducer(primitiveSize)) {
+ super.visitInsn(primitiveSize == 2 ? POP2 : POP);
+ }
+ }
+
+ /**
+ * Cancels a pure pending load/constant that produces a primitive of
+ * {@code primitiveSize} slots (the operand of a discarded box). Attached
+ * {@code CHECKCAST} and reference loads are never cancelled here.
+ *
+ * @return {@code true} if a producer was cancelled (nothing left on stack)
+ */
+ private boolean tryCancelPendingPrimitiveProducer(final int primitiveSize)
{
+ if (pendingLoadKind == PendingLoadKind.NONE
+ || pendingLoadKind == PendingLoadKind.CHECKCAST
+ || pendingCheckcastDescriptor != null) {
+ return false;
+ }
+ if (pendingLoadKind == PendingLoadKind.VARIABLE && pendingLoadOpcode
== ALOAD) {
+ return false;
+ }
+ if (stackSizeForPendingLoad() != primitiveSize) {
+ return false;
+ }
+ emitPreservedIincSideEffect();
+ clearPendingLoad();
return true;
}
@@ -1047,9 +1146,13 @@ public final class PeepholeOptimizingMethodVisitor
extends MethodVisitor {
/**
* Buffers {@code Wrapper.valueOf(prim)} or {@code
DefaultTypeTransformation.box(prim)}
- * so a following matching unbox (or discard via pop) can collapse the
pair.
- * Any previously pending state is flushed first so the primitive producer
is
- * already on the operand stack.
+ * so a following matching unbox (or discard via pop/return) can collapse
the pair.
+ * <p>
+ * When the load window already holds a pure primitive producer of the
right
+ * size, that load is kept pending under the box so {@code load}; {@code
box};
+ * {@code POP} can collapse to nothing. Otherwise the load (and any prior
box)
+ * is flushed so the primitive is on the operand stack before the box is
+ * recorded.
*
* @return {@code true} if the call was buffered
*/
@@ -1062,7 +1165,7 @@ public final class PeepholeOptimizingMethodVisitor
extends MethodVisitor {
if (primitive == null) {
return false;
}
- flushPending();
+ prepareBoxBuffer(primitive);
bufferValueOfBox(owner, primitive, isInterface);
return true;
}
@@ -1071,37 +1174,114 @@ public final class PeepholeOptimizingMethodVisitor
extends MethodVisitor {
if (primitive == null) {
return false;
}
- flushPending();
+ prepareBoxBuffer(primitive);
bufferDttBox(primitive);
return true;
}
return false;
}
+ /**
+ * Ensures the operand stack / load window is ready to record a new box of
+ * {@code primitiveDescriptor}. Keeps a matching pure pending load so a
later
+ * discard can eliminate both; flushes everything else.
+ */
+ private void prepareBoxBuffer(final String primitiveDescriptor) {
+ if (pendingBoxKind != PendingBoxKind.NONE) {
+ // Previous box must be emitted with its operand already on the
stack.
+ flushPendingLoad();
+ flushPendingDupStore();
+ flushPendingBox();
+ return;
+ }
+ if (!pendingLoadProducesPrimitive(primitiveDescriptor)) {
+ flushPendingLoad();
+ }
+ flushPendingDupStore();
+ }
+
+ /**
+ * {@code true} when the load window holds a pure primitive producer whose
+ * stack size matches {@code primitiveDescriptor} (suitable to keep under a
+ * pending box). Reference loads, casts, and size mismatches return
+ * {@code false}.
+ */
+ private boolean pendingLoadProducesPrimitive(final String
primitiveDescriptor) {
+ if (pendingLoadKind == PendingLoadKind.NONE
+ || pendingLoadKind == PendingLoadKind.CHECKCAST
+ || pendingCheckcastDescriptor != null) {
+ return false;
+ }
+ int need = stackSizeForPrimitiveDescriptor(primitiveDescriptor);
+ return switch (pendingLoadKind) {
+ case VARIABLE -> pendingLoadOpcode != ALOAD
+ && stackSizeForLoadOpcode(pendingLoadOpcode) == need;
+ case CONSTANT -> pendingConstant != null
+ && stackSizeForPendingLoad() == need
+ && constantCompatibleWithPrimitive(pendingConstant,
primitiveDescriptor);
+ case BOOLEAN_CONSTANT -> "Z".equals(primitiveDescriptor);
+ default -> false;
+ };
+ }
+
+ /**
+ * Whether a buffered constant can be the operand of a box for
+ * {@code primitiveDescriptor}. Size agreement is required; floating vs
+ * integral kinds must not mix.
+ */
+ private static boolean constantCompatibleWithPrimitive(final Object value,
final String primitiveDescriptor) {
+ return switch (primitiveDescriptor) {
+ case "Z", "B", "C", "S", "I" -> value instanceof Integer || value
instanceof Boolean;
+ case "J" -> value instanceof Long;
+ case "F" -> value instanceof Float;
+ case "D" -> value instanceof Double;
+ default -> false;
+ };
+ }
+
private boolean matchesPendingUnbox(final int opcode, final String owner,
final String name, final String descriptor) {
String expectedPrim = pendingBoxPrimitiveDescriptor;
- if (expectedPrim == null) {
+ if (expectedPrim == null || pendingBoxKind == PendingBoxKind.NONE) {
return false;
}
- if (pendingBoxKind == PendingBoxKind.VALUE_OF) {
- // Wrapper.xxxValue()X — e.g. Integer.intValue()I
- if (opcode != INVOKEVIRTUAL || !pendingBoxOwner.equals(owner)) {
- return false;
+ // Same-type Wrapper.xxxValue() — valid after valueOf or DTT.box.
+ if (isWrapperPrimitiveUnbox(opcode, owner, name, descriptor,
expectedPrim)) {
+ if (pendingBoxKind == PendingBoxKind.VALUE_OF) {
+ return pendingBoxOwner.equals(owner);
}
- String expectedName = primitiveValueMethodName(expectedPrim);
- String expectedDesc = "()" + expectedPrim;
- return expectedName != null && expectedName.equals(name) &&
expectedDesc.equals(descriptor);
+ // DTT.box → Object at the type system, but runtime value is the
wrapper.
+ return
owner.equals(wrapperInternalNameForDescriptor(expectedPrim));
}
- if (pendingBoxKind == PendingBoxKind.DTT_BOX) {
- // DefaultTypeTransformation.xxxUnbox(Object)X
- if (opcode != INVOKESTATIC || !DTT_OWNER.equals(owner)) {
- return false;
- }
- String expectedName = primitiveUnboxMethodName(expectedPrim);
- String expectedDesc = "(Ljava/lang/Object;)" + expectedPrim;
- return expectedName != null && expectedName.equals(name) &&
expectedDesc.equals(descriptor);
+ // Same-type DTT.xxxUnbox — valid after valueOf or DTT.box.
+ return isDttPrimitiveUnbox(opcode, owner, name, descriptor,
expectedPrim);
+ }
+
+ private static boolean isWrapperPrimitiveUnbox(final int opcode, final
String owner, final String name,
+ final String descriptor,
final String primitiveDescriptor) {
+ if (opcode != INVOKEVIRTUAL) {
+ return false;
}
- return false;
+ String expectedName = primitiveValueMethodName(primitiveDescriptor);
+ String expectedDesc = "()" + primitiveDescriptor;
+ return expectedName != null && expectedName.equals(name) &&
expectedDesc.equals(descriptor)
+ &&
owner.equals(wrapperInternalNameForDescriptor(primitiveDescriptor));
+ }
+
+ private static boolean isDttPrimitiveUnbox(final int opcode, final String
owner, final String name,
+ final String descriptor, final
String primitiveDescriptor) {
+ if (opcode != INVOKESTATIC || !DTT_OWNER.equals(owner)) {
+ return false;
+ }
+ String expectedName = primitiveUnboxMethodName(primitiveDescriptor);
+ String expectedDesc = "(Ljava/lang/Object;)" + primitiveDescriptor;
+ return expectedName != null && expectedName.equals(name) &&
expectedDesc.equals(descriptor);
+ }
+
+ private static String wrapperInternalNameForDescriptor(final String
primitiveDescriptor) {
+ if (primitiveDescriptor == null || primitiveDescriptor.length() != 1) {
+ return null;
+ }
+ return wrapperInternalName(Type.getType(primitiveDescriptor));
}
private static boolean isBooleanTrueFalse(final String owner, final String
name, final String descriptor) {
diff --git a/src/test/groovy/bugs/Groovy12162.groovy
b/src/test/groovy/bugs/Groovy12162.groovy
new file mode 100644
index 0000000000..ad91d4bd37
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy12162.groovy
@@ -0,0 +1,80 @@
+/*
+ * 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 bugs
+
+import org.codehaus.groovy.classgen.asm.AbstractBytecodeTestCase
+import org.junit.jupiter.api.Test
+
+import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
+
+/**
+ * GROOVY-12162: discarded casts must keep their type-check side effect.
+ * <p>
+ * Peephole compaction treats {@code CHECKCAST} before {@code POP} and before
+ * void {@code RETURN} the same way — the cast is kept. Opcode-level coverage
+ * lives in {@code PeepholeOptimizingMethodVisitorTest}; these tests pin the
+ * observable end-to-end behaviour under {@code @CompileStatic}.
+ */
+final class Groovy12162 extends AbstractBytecodeTestCase {
+
+ @Test
+ void discardedGroovyCastKeepsSideEffect() {
+ // SC emits invokedynamic cast (not bare CHECKCAST) for a general
Groovy
+ // cast; the call must not be dropped when the result is discarded.
+ def err =
shouldFail(org.codehaus.groovy.runtime.typehandling.GroovyCastException, '''
+ @groovy.transform.CompileStatic
+ void m(Object o) {
+ (Thread) o
+ }
+ m(42)
+ ''')
+ assert err.message.contains('Thread')
+ }
+
+ @Test
+ void discardedGroovyCastSucceedsWhenConversionApplies() {
+ assertScript '''
+ @groovy.transform.CompileStatic
+ void m(Object o) {
+ (String) o
+ }
+ m(42) // Integer → String via Groovy cast
+ m('ok')
+ '''
+ }
+
+ @Test
+ void discardedCheckcastAfterInstanceofIsPreservedInBytecode() {
+ // After instanceof String, SC emits a bare CHECKCAST for a discarded
+ // (String) o. The peephole must not erase that CHECKCAST (POP path).
+ def bytecode = compile(method: 'm', '''
+ @groovy.transform.CompileStatic
+ void m(Object o) {
+ if (o instanceof String) {
+ (String) o
+ }
+ }
+ ''')
+ assert bytecode.hasStrictSequence([
+ 'CHECKCAST java/lang/String',
+ 'POP',
+ ])
+ }
+}
diff --git
a/src/test/groovy/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitorTest.groovy
b/src/test/groovy/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitorTest.groovy
index ee2618a32c..8a1a29d972 100644
---
a/src/test/groovy/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitorTest.groovy
+++
b/src/test/groovy/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitorTest.groovy
@@ -330,7 +330,7 @@ final class PeepholeOptimizingMethodVisitorTest {
}
@Test
- void removesLoadAndAttachedCheckcastBeforePop() {
+ void preservesLoadAndAttachedCheckcastBeforePop() {
def bytecode = sequenceFor('(Ljava/lang/Object;)V') {
visitVarInsn(Opcodes.ALOAD, 0)
visitTypeInsn(Opcodes.CHECKCAST, 'java/lang/String')
@@ -338,12 +338,17 @@ final class PeepholeOptimizingMethodVisitorTest {
visitInsn(RETURN)
}
- // ALOAD + CHECKCAST are both dead once the value is discarded.
- assert opcodeLines(bytecode) == ['RETURN']
+ // Discarded cast must keep its ClassCastException side effect (same
rule as void RETURN).
+ assert opcodeLines(bytecode) == [
+ 'ALOAD 0',
+ 'CHECKCAST java/lang/String',
+ 'POP',
+ 'RETURN',
+ ]
}
@Test
- void removesStandaloneCheckcastBeforePop() {
+ void preservesStandaloneCheckcastBeforePop() {
def bytecode = sequenceFor('(Ljava/lang/Object;)V') {
visitVarInsn(Opcodes.ALOAD, 0)
visitInsn(Opcodes.NOP) // force the load to flush so CHECKCAST is
standalone
@@ -352,7 +357,14 @@ final class PeepholeOptimizingMethodVisitorTest {
visitInsn(RETURN)
}
- assert opcodeLines(bytecode) == ['ALOAD 0', 'NOP', 'POP', 'RETURN']
+ // Standalone cast is preserved; ALOAD is already flushed (NOP
boundary), so it stays.
+ assert opcodeLines(bytecode) == [
+ 'ALOAD 0',
+ 'NOP',
+ 'CHECKCAST java/lang/String',
+ 'POP',
+ 'RETURN',
+ ]
}
@Test
@@ -443,16 +455,14 @@ final class PeepholeOptimizingMethodVisitorTest {
@Test
void preservesStandaloneCheckcastBeforeReturn() {
- def bytecode = sequenceFor('(Ljava/lang/Object;)V') {
+ // POP and void RETURN use the same rule: keep CHECKCAST, discard the
value.
+ def withPop = sequenceFor('(Ljava/lang/Object;)V') {
visitVarInsn(Opcodes.ALOAD, 0)
- visitInsn(Opcodes.NOP)
+ visitInsn(Opcodes.NOP) // flush ALOAD so the cast is standalone
visitTypeInsn(Opcodes.CHECKCAST, 'java/lang/String')
visitInsn(Opcodes.POP)
visitInsn(RETURN)
}
-
- // Standalone CHECKCAST before POP is dropped; before RETURN it is
flushed
- // and the value is popped so the void return remains verifiable.
def returnOnly = sequenceFor('(Ljava/lang/Object;)V') {
visitVarInsn(Opcodes.ALOAD, 0)
visitInsn(Opcodes.NOP)
@@ -460,8 +470,20 @@ final class PeepholeOptimizingMethodVisitorTest {
visitInsn(RETURN)
}
- assert opcodeLines(bytecode) == ['ALOAD 0', 'NOP', 'POP', 'RETURN']
- assert opcodeLines(returnOnly) == ['ALOAD 0', 'NOP', 'CHECKCAST
java/lang/String', 'POP', 'RETURN']
+ assert opcodeLines(withPop) == [
+ 'ALOAD 0',
+ 'NOP',
+ 'CHECKCAST java/lang/String',
+ 'POP',
+ 'RETURN',
+ ]
+ assert opcodeLines(returnOnly) == [
+ 'ALOAD 0',
+ 'NOP',
+ 'CHECKCAST java/lang/String',
+ 'POP',
+ 'RETURN',
+ ]
}
@Test
@@ -592,7 +614,7 @@ final class PeepholeOptimizingMethodVisitorTest {
}
@Test
- void attachesCheckcastToReferenceConstantsAndDropsDeadCastChains() {
+ void attachesCheckcastToReferenceConstantsAndPreservesDiscardedCasts() {
def type = org.objectweb.asm.Type.getType(String)
def handle = new Handle(Opcodes.H_INVOKESTATIC, 'Owner', 'boot',
'()V', false)
def bytecode = sequenceFor {
@@ -611,7 +633,22 @@ final class PeepholeOptimizingMethodVisitorTest {
visitInsn(RETURN)
}
- assert opcodeLines(bytecode) == ['RETURN']
+ // Casts attach to reference constants but are kept on discard (CCE
side effect).
+ assert opcodeLines(bytecode) == [
+ 'LDC "text"',
+ 'CHECKCAST java/lang/String',
+ 'POP',
+ 'LDC Ljava/lang/String;.class',
+ 'CHECKCAST java/lang/Class',
+ 'POP',
+ 'LDC Owner.boot()V',
+ 'CHECKCAST java/lang/invoke/MethodHandle',
+ 'POP',
+ 'ACONST_NULL',
+ 'CHECKCAST java/lang/String',
+ 'POP',
+ 'RETURN',
+ ]
}
@Test
@@ -865,20 +902,27 @@ final class PeepholeOptimizingMethodVisitorTest {
}
@Test
- void dropsDeadLoadWithAttachedCheckcastOnPopOnly() {
+ void preservesAttachedCheckcastOnConstantBeforePop() {
def withPop = sequenceFor('(Ljava/lang/Object;)V') {
visitLdcInsn('x')
visitTypeInsn(Opcodes.CHECKCAST, 'java/lang/String')
visitInsn(Opcodes.POP)
visitInsn(RETURN)
}
- assert opcodeLines(withPop) == ['RETURN']
+ // Cast side effect retained even for a reference constant (consistent
with ALOAD path).
+ assert opcodeLines(withPop) == [
+ 'LDC "x"',
+ 'CHECKCAST java/lang/String',
+ 'POP',
+ 'RETURN',
+ ]
+ // POP2 does not match a one-slot cast result; cast is flushed
unchanged with POP2.
def standalonePop2 = sequenceFor('(Ljava/lang/Object;)V') {
visitVarInsn(Opcodes.ALOAD, 0)
- visitInsn(Opcodes.NOP)
+ visitInsn(Opcodes.NOP) // flush ALOAD so the cast is standalone
visitTypeInsn(Opcodes.CHECKCAST, 'java/lang/String')
- visitInsn(Opcodes.POP2) // only POP removes standalone cast
+ visitInsn(Opcodes.POP2)
visitInsn(RETURN)
}
assert opcodeLines(standalonePop2) == [
@@ -989,7 +1033,8 @@ final class PeepholeOptimizingMethodVisitorTest {
visitInsn(RETURN)
}
- assert opcodeLines(bytecode) == ['ILOAD 0', 'POP', 'RETURN']
+ // Box and pure ILOAD are both dead once the boxed value is discarded.
+ assert opcodeLines(bytecode) == ['RETURN']
}
@Test
@@ -1001,7 +1046,7 @@ final class PeepholeOptimizingMethodVisitorTest {
visitInsn(RETURN)
}
- assert opcodeLines(bytecode) == ['LLOAD 0', 'POP2', 'RETURN']
+ assert opcodeLines(bytecode) == ['RETURN']
}
@Test
@@ -1015,26 +1060,115 @@ final class PeepholeOptimizingMethodVisitorTest {
visitInsn(RETURN)
}
- // POP2 size matches the wide primitive that was boxed → drop box, pop
the double.
- assert opcodeLines(bytecode) == ['DLOAD 0', 'POP2', 'RETURN']
+ // POP2 matches the wide primitive under the box → drop box and
producer.
+ assert opcodeLines(bytecode) == ['RETURN']
}
@Test
- void doesNotTreatPop2AsDiscardOfNarrowBoxedValue() {
+ void treatsPop2OnNarrowBoxedValueAsTwoSlotDiscard() {
+ // Stack before POP2: [underneath, boxed]. Cancel pure ILOAD under the
box, POP the underneath.
def bytecode = sequenceFor('(I)V') {
+ visitInsn(Opcodes.ICONST_0)
visitVarInsn(Opcodes.ILOAD, 0)
visitMethodInsn(Opcodes.INVOKESTATIC, 'java/lang/Integer',
'valueOf', '(I)Ljava/lang/Integer;', false)
visitInsn(Opcodes.POP2)
visitInsn(RETURN)
}
- // POP2 does not match a one-slot boxed reference; flush the box and
keep POP2.
- assert opcodeLines(bytecode) == [
- 'ILOAD 0',
- 'INVOKESTATIC java/lang/Integer.valueOf
(I)Ljava/lang/Integer;',
- 'POP2',
- 'RETURN',
- ]
+ assert opcodeLines(bytecode) == ['ICONST_0', 'POP', 'RETURN']
+ }
+
+ @Test
+ void dropsBoxedValueDiscardedByVoidReturn() {
+ def bytecode = sequenceFor('(I)V') {
+ visitVarInsn(Opcodes.ILOAD, 0)
+ visitMethodInsn(Opcodes.INVOKESTATIC, 'java/lang/Integer',
'valueOf', '(I)Ljava/lang/Integer;', false)
+ visitInsn(RETURN)
+ }
+
+ assert opcodeLines(bytecode) == ['RETURN']
+ }
+
+ @Test
+ void dropsBoxedConstantDiscardedByPop() {
+ def bytecode = sequenceFor {
+ visitInsn(Opcodes.ICONST_1)
+ visitMethodInsn(Opcodes.INVOKESTATIC, 'java/lang/Integer',
'valueOf', '(I)Ljava/lang/Integer;', false)
+ visitInsn(Opcodes.POP)
+ visitInsn(RETURN)
+ }
+
+ assert opcodeLines(bytecode) == ['RETURN']
+ }
+
+ @Test
+ void preservesIincWhenBoxedLoadIsDiscarded() {
+ def bytecode = sequenceFor('(I)V') {
+ visitVarInsn(Opcodes.ILOAD, 0)
+ visitIincInsn(0, 1)
+ visitMethodInsn(Opcodes.INVOKESTATIC, 'java/lang/Integer',
'valueOf', '(I)Ljava/lang/Integer;', false)
+ visitInsn(Opcodes.POP)
+ visitInsn(RETURN)
+ }
+
+ // IINC is a side effect of the load window and must survive dead-box
elimination.
+ assert opcodeLines(bytecode) == ['IINC 0 1', 'RETURN']
+ }
+
+ @Test
+ void popsPrimitiveWhenBoxedProducerAlreadyFlushed() {
+ // NOP flushes ILOAD before valueOf is seen, so only the box is
pending.
+ def bytecode = sequenceFor('(I)V') {
+ visitVarInsn(Opcodes.ILOAD, 0)
+ visitInsn(Opcodes.NOP)
+ visitMethodInsn(Opcodes.INVOKESTATIC, 'java/lang/Integer',
'valueOf', '(I)Ljava/lang/Integer;', false)
+ visitInsn(Opcodes.POP)
+ visitInsn(RETURN)
+ }
+
+ assert opcodeLines(bytecode) == ['ILOAD 0', 'NOP', 'POP', 'RETURN']
+ }
+
+ @Test
+ void popsWidePrimitiveWhenBoxedProducerAlreadyFlushed() {
+ def bytecode = sequenceFor('(J)V') {
+ visitVarInsn(Opcodes.LLOAD, 0)
+ visitInsn(Opcodes.NOP)
+ visitMethodInsn(Opcodes.INVOKESTATIC, 'java/lang/Long', 'valueOf',
'(J)Ljava/lang/Long;', false)
+ visitInsn(Opcodes.POP)
+ visitInsn(RETURN)
+ }
+
+ // POP of boxed long → POP2 of the long still on the stack.
+ assert opcodeLines(bytecode) == ['LLOAD 0', 'NOP', 'POP2', 'RETURN']
+ }
+
+ @Test
+ void treatsPop2OnFlushedNarrowBoxAsTwoSlotDiscard() {
+ def bytecode = sequenceFor('(I)V') {
+ visitInsn(Opcodes.ICONST_0)
+ visitVarInsn(Opcodes.ILOAD, 0)
+ visitInsn(Opcodes.NOP) // flush ILOAD; int remains on stack under
the later box
+ visitMethodInsn(Opcodes.INVOKESTATIC, 'java/lang/Integer',
'valueOf', '(I)Ljava/lang/Integer;', false)
+ visitInsn(Opcodes.POP2)
+ visitInsn(RETURN)
+ }
+
+ // Drop box; POP2 removes the flushed int and the ICONST underneath.
+ assert opcodeLines(bytecode) == ['ICONST_0', 'ILOAD 0', 'NOP', 'POP2',
'RETURN']
+ }
+
+ @Test
+ void cancelsBoxUnboxAfterFlushedProducer() {
+ def bytecode = sequenceFor('(I)I') {
+ visitVarInsn(Opcodes.ILOAD, 0)
+ visitInsn(Opcodes.NOP)
+ visitMethodInsn(Opcodes.INVOKESTATIC, 'java/lang/Integer',
'valueOf', '(I)Ljava/lang/Integer;', false)
+ visitMethodInsn(Opcodes.INVOKEVIRTUAL, 'java/lang/Integer',
'intValue', '()I', false)
+ visitInsn(Opcodes.IRETURN)
+ }
+
+ assert opcodeLines(bytecode) == ['ILOAD 0', 'NOP', 'IRETURN']
}
@Test
@@ -1185,8 +1319,8 @@ final class PeepholeOptimizingMethodVisitorTest {
}
@Test
- void doesNotCancelDttBoxWithValueOfStyleUnbox() {
- // DTT.box then Integer.intValue — different unbox convention, must
not cancel.
+ void cancelsDttBoxWithMatchingWrapperUnbox() {
+ // Same primitive type: DTT.box then Integer.intValue is an identity
on the int.
def bytecode = sequenceFor('(I)I') {
visitVarInsn(Opcodes.ILOAD, 0)
visitMethodInsn(Opcodes.INVOKESTATIC,
@@ -1196,11 +1330,40 @@ final class PeepholeOptimizingMethodVisitorTest {
visitInsn(Opcodes.IRETURN)
}
+ assert opcodeLines(bytecode) == ['ILOAD 0', 'IRETURN']
+ }
+
+ @Test
+ void cancelsValueOfWithMatchingDttUnbox() {
+ def bytecode = sequenceFor('(I)I') {
+ visitVarInsn(Opcodes.ILOAD, 0)
+ visitMethodInsn(Opcodes.INVOKESTATIC, 'java/lang/Integer',
'valueOf', '(I)Ljava/lang/Integer;', false)
+ visitMethodInsn(Opcodes.INVOKESTATIC,
+
'org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation',
+ 'intUnbox', '(Ljava/lang/Object;)I', false)
+ visitInsn(Opcodes.IRETURN)
+ }
+
+ assert opcodeLines(bytecode) == ['ILOAD 0', 'IRETURN']
+ }
+
+ @Test
+ void doesNotCancelDttBoxWithMismatchedWrapperUnbox() {
+ // DTT.box(int) then Long.longValue — different primitive type, must
not cancel.
+ def bytecode = sequenceFor('(I)J') {
+ visitVarInsn(Opcodes.ILOAD, 0)
+ visitMethodInsn(Opcodes.INVOKESTATIC,
+
'org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation',
+ 'box', '(I)Ljava/lang/Object;', false)
+ visitMethodInsn(Opcodes.INVOKEVIRTUAL, 'java/lang/Long',
'longValue', '()J', false)
+ visitInsn(Opcodes.LRETURN)
+ }
+
assert opcodeLines(bytecode) == [
'ILOAD 0',
'INVOKESTATIC
org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.box
(I)Ljava/lang/Object;',
- 'INVOKEVIRTUAL java/lang/Integer.intValue ()I',
- 'IRETURN',
+ 'INVOKEVIRTUAL java/lang/Long.longValue ()J',
+ 'LRETURN',
]
}
