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 c222d71969 GROOVY-11415: Tweak bytecode for identity c222d71969 is described below commit c222d7196925717708170fd8652d81565049add8 Author: Daniel Sun <sun...@apache.org> AuthorDate: Sat Jun 22 10:10:06 2024 +0900 GROOVY-11415: Tweak bytecode for identity --- .../classgen/asm/BinaryExpressionHelper.java | 32 ++++++++++-- src/test-resources/core/IdenticalOp_01x.groovy | 61 ++++++++++++++++++---- src/test/groovy/bugs/Groovy11415.groovy | 53 +++++++++++++++++++ 3 files changed, 131 insertions(+), 15 deletions(-) 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 dd45781aa6..e9beddd1af 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java @@ -118,13 +118,12 @@ import static org.objectweb.asm.Opcodes.GOTO; import static org.objectweb.asm.Opcodes.IFEQ; import static org.objectweb.asm.Opcodes.IFNE; import static org.objectweb.asm.Opcodes.IF_ACMPEQ; +import static org.objectweb.asm.Opcodes.IF_ACMPNE; import static org.objectweb.asm.Opcodes.INSTANCEOF; import static org.objectweb.asm.Opcodes.POP; public class BinaryExpressionHelper { // compare - private static final MethodCaller compareIdenticalMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "compareIdentical"); - private static final MethodCaller compareNotIdenticalMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "compareNotIdentical"); private static final MethodCaller compareEqualMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "compareEqual"); private static final MethodCaller compareNotEqualMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "compareNotEqual"); private static final MethodCaller compareToMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "compareTo"); @@ -352,11 +351,11 @@ public class BinaryExpressionHelper { break; case COMPARE_IDENTICAL: - evaluateCompareExpression(compareIdenticalMethod, expression); + evaluateIdentity(expression, true); break; case COMPARE_NOT_IDENTICAL: - evaluateCompareExpression(compareNotIdenticalMethod, expression); + evaluateIdentity(expression, false); break; default: @@ -364,6 +363,31 @@ public class BinaryExpressionHelper { } } + private void evaluateIdentity(BinaryExpression expression, boolean identical) { + AsmClassGenerator acg = controller.getAcg(); + MethodVisitor mv = controller.getMethodVisitor(); + OperandStack operandStack = controller.getOperandStack(); + + Expression lhs = expression.getLeftExpression(); + lhs.visit(acg); + if (ClassHelper.isPrimitiveType(lhs.getType())) operandStack.box(); + + Expression rhs = expression.getRightExpression(); + rhs.visit(acg); + if (ClassHelper.isPrimitiveType(rhs.getType())) operandStack.box(); + + Label trueCase = operandStack.jump(identical ? IF_ACMPEQ : IF_ACMPNE); + ConstantExpression.PRIM_FALSE.visit(acg); + Label end = new Label(); + mv.visitJumpInsn(GOTO, end); + + mv.visitLabel(trueCase); + ConstantExpression.PRIM_TRUE.visit(acg); + + mv.visitLabel(end); + operandStack.replace(ClassHelper.boolean_TYPE, 3); + } + @Deprecated protected void assignToArray(final Expression parent, final Expression receiver, final Expression index, final Expression rhsValueLoader) { assignToArray(parent, receiver, index, rhsValueLoader, false); diff --git a/src/test-resources/core/IdenticalOp_01x.groovy b/src/test-resources/core/IdenticalOp_01x.groovy index 6703efb5cb..9efe74daca 100644 --- a/src/test-resources/core/IdenticalOp_01x.groovy +++ b/src/test-resources/core/IdenticalOp_01x.groovy @@ -1,3 +1,5 @@ +import groovy.transform.CompileStatic + /* * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -16,15 +18,52 @@ * specific language governing permissions and limitations * under the License. */ -def x = [] -def y = [] -assert y !== x -assert 'a' === 'a' -def otherA = new String('a') -assert 'a' == otherA && 'a'.equals(otherA) -assert 'a' !== otherA && !'a'.is(otherA) -assert null === null -assert true === true -assert false === false -assert 0 == 0 && 'a' === 'a' && 1 != 2 && 'a' !== new String('a') && 1 == 1 \ No newline at end of file +def c() { + def x = [] + def y = [] + assert y !== x + assert 'a' === 'a' + def otherA = new String('a') + assert 'a' == otherA && 'a'.equals(otherA) + assert 'a' !== otherA && !'a'.is(otherA) + assert 0 == 0 && 'a' === 'a' && 1 != 2 && 'a' !== new String('a') && 1 == 1 + + def a = 'abc' + def b = a + assert a === b + assert !(a !== b) + assert a === b && !(a !== b) + assert null === null + assert true === true + assert false === false + assert 0 === 0 + assert 0 !== null + assert null !== 0 +} +c() + +@CompileStatic +def c_cs() { + def x = [] + def y = [] + assert y !== x + assert 'a' === 'a' + def otherA = new String('a') + assert 'a' == otherA && 'a'.equals(otherA) + assert 'a' !== otherA && !'a'.is(otherA) + assert 0 == 0 && 'a' === 'a' && 1 != 2 && 'a' !== new String('a') && 1 == 1 + + def a = 'abc' + def b = a + assert a === b + assert !(a !== b) + assert a === b && !(a !== b) + assert null === null + assert true === true + assert false === false + assert 0 === 0 + assert 0 !== null + assert null !== 0 +} +c_cs() diff --git a/src/test/groovy/bugs/Groovy11415.groovy b/src/test/groovy/bugs/Groovy11415.groovy new file mode 100644 index 0000000000..47864b3166 --- /dev/null +++ b/src/test/groovy/bugs/Groovy11415.groovy @@ -0,0 +1,53 @@ +/* + * 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 static groovy.test.GroovyAssert.isAtLeastJdk + +final class Groovy11415 extends AbstractBytecodeTestCase { + void testIdentity() { + assert compile(method: 'isIdentical', ''' + def isIdentical (Object obj) { + if (obj === 'abc') return true + return false + } + ''').hasSequence([ + 'LDC "abc"', + 'IF_ACMPEQ L1', + 'ICONST_0', + 'GOTO L2' + ]) + } + + void testNotIdentity() { + assert compile(method: 'isNotIdentical', ''' + def isNotIdentical (Object obj) { + if (obj !== 'abc') return true + return false + } + ''').hasSequence([ + 'LDC "abc"', + 'IF_ACMPNE L1', + 'ICONST_0', + 'GOTO L2' + ]) + } +}