You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2021/11/21 17:52:00 UTC
[groovy] branch master updated: GROOVY-10377: SC: optimize `x === y` and `x !== y` for non-primitives
This is an automated email from the ASF dual-hosted git repository.
emilles 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 595f7e7 GROOVY-10377: SC: optimize `x === y` and `x !== y` for non-primitives
595f7e7 is described below
commit 595f7e78c1d4b1f360f04322ea56008cb1933dc0
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Nov 21 11:03:16 2021 -0600
GROOVY-10377: SC: optimize `x === y` and `x !== y` for non-primitives
---
.../transformers/BinaryExpressionTransformer.java | 51 +++++++++-------
.../sc/transformers/CompareIdentityExpression.java | 68 +++++++++++++---------
.../sc/transformers/CompareToNullExpression.java | 43 +++++++-------
3 files changed, 93 insertions(+), 69 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java
index 2e609cd..53803c1 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java
@@ -92,11 +92,11 @@ public class BinaryExpressionTransformer {
if (c != null)
return transformCharacterInitialization(bin, c);
}
- // for "float|double|BigDecimal x = n" change n's type
- if (!rightExpression.getType().equals(declarationType)
- && ClassHelper.getWrapper(declarationType).isDerivedFrom(ClassHelper.Number_TYPE)
- && WideningCategories.isDoubleCategory(ClassHelper.getUnwrapper(declarationType))) {
- return transformFloatingPointInitialization(bin, (Number) ((ConstantExpression) rightExpression).getValue(), declarationType);
+ // for "int|long|short|byte|char|float|double|BigDecimal|BigInteger x = n" change n's type
+ if (!declarationType.equals(rightExpression.getType())
+ && WideningCategories.isDoubleCategory(ClassHelper.getUnwrapper(declarationType))
+ && ClassHelper.getWrapper(rightExpression.getType()).isDerivedFrom(ClassHelper.Number_TYPE)) {
+ return transformNumericalInitialization(bin, (Number) ((ConstantExpression) rightExpression).getValue(), declarationType);
}
}
@@ -138,7 +138,7 @@ public class BinaryExpressionTransformer {
return bin;
}
- private Expression transformFloatingPointInitialization(final BinaryExpression bin, final Number rhs, final ClassNode lhsType) {
+ private Expression transformNumericalInitialization(final BinaryExpression bin, final Number rhs, final ClassNode lhsType) {
Expression ce = constX(convertConstant(rhs, ClassHelper.getWrapper(lhsType)), true);
ce.setSourcePosition(bin.getRightExpression());
ce.setType(lhsType);
@@ -244,16 +244,24 @@ public class BinaryExpressionTransformer {
}
private Expression transformEqualityComparison(final BinaryExpression bin, final boolean eq) {
- if (isNullConstant(bin.getRightExpression())) {
- Expression ctn = new CompareToNullExpression(staticCompilationTransformer.transform(bin.getLeftExpression()), eq);
+ Expression leftExpression = bin.getLeftExpression(), rightExpression = bin.getRightExpression();
+ if (isNullConstant(rightExpression)) {
+ Expression ctn = new CompareToNullExpression(staticCompilationTransformer.transform(leftExpression), eq);
ctn.setSourcePosition(bin);
return ctn;
}
- if (isNullConstant(bin.getLeftExpression())) {
- Expression ctn = new CompareToNullExpression(staticCompilationTransformer.transform(bin.getRightExpression()), eq);
+ if (isNullConstant(leftExpression)) {
+ Expression ctn = new CompareToNullExpression(staticCompilationTransformer.transform(rightExpression), eq);
ctn.setSourcePosition(bin);
return ctn;
}
+ if (bin.getOperation().getText().length() == 3
+ && !ClassHelper.isPrimitiveType(findType(leftExpression))
+ && !ClassHelper.isPrimitiveType(findType(rightExpression))) {
+ Expression cid = new CompareIdentityExpression(staticCompilationTransformer.transform(leftExpression), eq, staticCompilationTransformer.transform(rightExpression));
+ cid.setSourcePosition(bin);
+ return cid;
+ }
return null;
}
@@ -408,7 +416,7 @@ public class BinaryExpressionTransformer {
return staticCompilationTransformer.getTypeChooser().resolveType(e, staticCompilationTransformer.getClassNode());
}
- private boolean hasCharType(final Expression e) {
+ private static boolean hasCharType(final Expression e) {
ClassNode inferredType = e.getNodeMetaData(StaticTypesMarker.INFERRED_RETURN_TYPE); //TODO:findType(e);
return inferredType != null && ClassHelper.getWrapper(inferredType).equals(ClassHelper.Character_TYPE);
}
@@ -424,30 +432,33 @@ public class BinaryExpressionTransformer {
}
private static Object convertConstant(final Number source, final ClassNode target) {
- if (ClassHelper.isWrapperByte(target)) {
- return source.byteValue();
- }
- if (ClassHelper.isWrapperShort(target)) {
- return source.shortValue();
- }
if (ClassHelper.isWrapperInteger(target)) {
return source.intValue();
}
if (ClassHelper.isWrapperLong(target)) {
return source.longValue();
}
+ if (ClassHelper.isWrapperByte(target)) {
+ return source.byteValue();
+ }
+ if (ClassHelper.isWrapperShort(target)) {
+ return source.shortValue();
+ }
if (ClassHelper.isWrapperFloat(target)) {
return source.floatValue();
}
if (ClassHelper.isWrapperDouble(target)) {
return source.doubleValue();
}
- if (ClassHelper.isBigIntegerType(target)) {
- return DefaultGroovyMethods.asType(source, BigInteger.class);
+ if (ClassHelper.isWrapperCharacter(target)) {
+ return (char) source.intValue();
}
if (ClassHelper.isBigDecimalType(target)) {
return DefaultGroovyMethods.asType(source, BigDecimal.class);
}
- throw new IllegalArgumentException("Unsupported numerical conversion");
+ if (ClassHelper.isBigIntegerType(target)) {
+ return DefaultGroovyMethods.asType(source, BigInteger.class);
+ }
+ throw new IllegalArgumentException("Unsupported conversion: " + target.getText());
}
}
diff --git a/src/main/java/org/codehaus/groovy/transform/sc/transformers/CompareIdentityExpression.java b/src/main/java/org/codehaus/groovy/transform/sc/transformers/CompareIdentityExpression.java
index 8ce4e5c..eb22463 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/transformers/CompareIdentityExpression.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/transformers/CompareIdentityExpression.java
@@ -19,6 +19,7 @@
package org.codehaus.groovy.transform.sc.transformers;
import org.codehaus.groovy.ast.ClassHelper;
+import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.GroovyCodeVisitor;
import org.codehaus.groovy.ast.expr.BinaryExpression;
import org.codehaus.groovy.ast.expr.Expression;
@@ -26,13 +27,13 @@ import org.codehaus.groovy.ast.expr.ExpressionTransformer;
import org.codehaus.groovy.classgen.AsmClassGenerator;
import org.codehaus.groovy.classgen.asm.WriterController;
import org.codehaus.groovy.syntax.Token;
-import org.codehaus.groovy.syntax.Types;
import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
import static org.objectweb.asm.Opcodes.GOTO;
import static org.objectweb.asm.Opcodes.ICONST_0;
import static org.objectweb.asm.Opcodes.ICONST_1;
+import static org.objectweb.asm.Opcodes.IF_ACMPEQ;
import static org.objectweb.asm.Opcodes.IF_ACMPNE;
/**
@@ -44,43 +45,56 @@ import static org.objectweb.asm.Opcodes.IF_ACMPNE;
* in the context of reference equality check.
*/
public class CompareIdentityExpression extends BinaryExpression {
- private final Expression leftExpression;
- private final Expression rightExpression;
+
+ public CompareIdentityExpression(final Expression leftExpression, final boolean eq, final Expression rightExpression) {
+ super(leftExpression, Token.newSymbol(eq ? "===" : "!==", -1, -1), rightExpression);
+ super.setType(ClassHelper.boolean_TYPE);
+ }
public CompareIdentityExpression(final Expression leftExpression, final Expression rightExpression) {
- super(leftExpression, new Token(Types.COMPARE_TO, "==", -1, -1), rightExpression);
- this.leftExpression = leftExpression;
- this.rightExpression = rightExpression;
+ this(leftExpression, true, rightExpression);
+ }
+
+ public boolean isEq() {
+ return getOperation().getText().charAt(0) == '=';
+ }
+
+ @Override
+ public void setType(final ClassNode type) {
+ throw new UnsupportedOperationException();
}
@Override
public Expression transformExpression(final ExpressionTransformer transformer) {
- return this;
+ Expression ret = new CompareIdentityExpression(transformer.transform(getLeftExpression()), isEq(), transformer.transform(getRightExpression()));
+ ret.setSourcePosition(this);
+ ret.copyNodeMetaData(this);
+ return ret;
}
@Override
public void visit(final GroovyCodeVisitor visitor) {
- if (visitor instanceof AsmClassGenerator) {
- AsmClassGenerator acg = (AsmClassGenerator) visitor;
- WriterController controller = acg.getController();
- controller.getTypeChooser().resolveType(leftExpression, controller.getClassNode());
- controller.getTypeChooser().resolveType(rightExpression, controller.getClassNode());
- MethodVisitor mv = controller.getMethodVisitor();
- leftExpression.visit(acg);
- controller.getOperandStack().box();
- rightExpression.visit(acg);
- controller.getOperandStack().box();
- Label l1 = new Label();
- mv.visitJumpInsn(IF_ACMPNE, l1);
- mv.visitInsn(ICONST_1);
- Label l2 = new Label();
- mv.visitJumpInsn(GOTO, l2);
- mv.visitLabel(l1);
- mv.visitInsn(ICONST_0);
- mv.visitLabel(l2);
- controller.getOperandStack().replace(ClassHelper.boolean_TYPE, 2);
- } else {
+ if (!(visitor instanceof AsmClassGenerator)) {
super.visit(visitor);
+ return;
}
+
+ WriterController controller = ((AsmClassGenerator) visitor).getController();
+ MethodVisitor mv = controller.getMethodVisitor();
+ Label no = new Label(), yes = new Label();
+
+ getLeftExpression().visit(visitor);
+ controller.getOperandStack().box();
+ getRightExpression().visit(visitor);
+ controller.getOperandStack().box();
+
+ mv.visitJumpInsn(isEq() ? IF_ACMPNE : IF_ACMPEQ, no);
+ mv.visitInsn(ICONST_1);
+ mv.visitJumpInsn(GOTO, yes);
+ mv.visitLabel(no);
+ mv.visitInsn(ICONST_0);
+ mv.visitLabel(yes);
+
+ controller.getOperandStack().replace(ClassHelper.boolean_TYPE, 2);
}
}
diff --git a/src/main/java/org/codehaus/groovy/transform/sc/transformers/CompareToNullExpression.java b/src/main/java/org/codehaus/groovy/transform/sc/transformers/CompareToNullExpression.java
index 6e1cecd..36e6436 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/transformers/CompareToNullExpression.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/transformers/CompareToNullExpression.java
@@ -74,32 +74,31 @@ public class CompareToNullExpression extends BinaryExpression {
@Override
public void visit(final GroovyCodeVisitor visitor) {
- if (visitor instanceof AsmClassGenerator) {
- AsmClassGenerator acg = (AsmClassGenerator) visitor;
- WriterController controller = acg.getController();
- MethodVisitor mv = controller.getMethodVisitor();
-
- getObjectExpression().visit(acg);
+ if (!(visitor instanceof AsmClassGenerator)) {
+ super.visit(visitor);
+ return;
+ }
- if (ClassHelper.isPrimitiveType(controller.getOperandStack().getTopOperand())) {
- controller.getOperandStack().pop();
- mv.visitInsn(equalsNull ? ICONST_0 : ICONST_1);
+ WriterController controller = ((AsmClassGenerator) visitor).getController();
+ MethodVisitor mv = controller.getMethodVisitor();
- controller.getOperandStack().push(ClassHelper.boolean_TYPE);
- } else {
- Label zero = new Label();
- mv.visitJumpInsn(equalsNull ? IFNONNULL : IFNULL, zero);
- mv.visitInsn(ICONST_1);
- Label end = new Label();
- mv.visitJumpInsn(GOTO, end);
- mv.visitLabel(zero);
- mv.visitInsn(ICONST_0);
- mv.visitLabel(end);
+ getObjectExpression().visit(visitor);
- controller.getOperandStack().replace(ClassHelper.boolean_TYPE);
- }
+ if (ClassHelper.isPrimitiveType(controller.getOperandStack().getTopOperand())) {
+ controller.getOperandStack().pop();
+ mv.visitInsn(equalsNull ? ICONST_0 : ICONST_1);
+ controller.getOperandStack().push(ClassHelper.boolean_TYPE);
} else {
- super.visit(visitor);
+ Label no = new Label(), yes = new Label();
+
+ mv.visitJumpInsn(equalsNull ? IFNONNULL : IFNULL, no);
+ mv.visitInsn(ICONST_1);
+ mv.visitJumpInsn(GOTO, yes);
+ mv.visitLabel(no);
+ mv.visitInsn(ICONST_0);
+ mv.visitLabel(yes);
+
+ controller.getOperandStack().replace(ClassHelper.boolean_TYPE);
}
}
}