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/19 22:29:54 UTC
[groovy] branch master updated: GROOVY-7473: SC: change `a in b` into `b.isCase(a)` for obvious non-null
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 5174938 GROOVY-7473: SC: change `a in b` into `b.isCase(a)` for obvious non-null
5174938 is described below
commit 51749380ff43d1cdfb3164c3a9e858c9d5573b9d
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri Nov 19 16:16:10 2021 -0600
GROOVY-7473: SC: change `a in b` into `b.isCase(a)` for obvious non-null
then `b == null ? a == null : b.isCase(a)` when null is a possibility
---
.../transformers/BinaryExpressionTransformer.java | 39 +++++++++++-----------
1 file changed, 20 insertions(+), 19 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 2948187..3dfd7c8 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
@@ -28,8 +28,10 @@ import org.codehaus.groovy.ast.expr.ConstantExpression;
import org.codehaus.groovy.ast.expr.DeclarationExpression;
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.ListExpression;
+import org.codehaus.groovy.ast.expr.MapExpression;
import org.codehaus.groovy.ast.expr.MethodCallExpression;
import org.codehaus.groovy.ast.expr.PropertyExpression;
+import org.codehaus.groovy.ast.expr.RangeExpression;
import org.codehaus.groovy.ast.expr.TupleExpression;
import org.codehaus.groovy.ast.expr.VariableExpression;
import org.codehaus.groovy.ast.tools.WideningCategories;
@@ -53,12 +55,11 @@ import java.util.List;
import static org.apache.groovy.ast.tools.ExpressionUtils.isNullConstant;
import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
import static org.codehaus.groovy.ast.tools.GeneralUtils.binX;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.boolX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.classX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.constX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.isNullX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.isOrImplements;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.nullX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.ternaryX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
import static org.codehaus.groovy.ast.ClassHelper.isBigDecimalType;
@@ -169,7 +170,20 @@ public class BinaryExpressionTransformer {
return compareToNullExpression;
}
} else if (operationType == Types.KEYWORD_IN) {
- return staticCompilationTransformer.transform(convertInOperatorToTernary(bin, rightExpression, leftExpression));
+ // transform "left in right" into "right.isCase(left)"
+ MethodCallExpression call = callX(rightExpression, "isCase", leftExpression);
+ call.setImplicitThis(false); call.setSourcePosition(bin); call.copyNodeMetaData(bin);
+ call.setMethodTarget(bin.getNodeMetaData(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET));
+ // GROOVY-7473: no null test for simple cases
+ if (rightExpression instanceof ListExpression
+ || rightExpression instanceof MapExpression
+ || rightExpression instanceof RangeExpression
+ || rightExpression instanceof ConstantExpression
+ && !isNullConstant(rightExpression))
+ return staticCompilationTransformer.transform(call);
+
+ // then "right == null ? left == null : right.isCase(left)" for null safety
+ return staticCompilationTransformer.transform(ternaryX(isNullX(rightExpression), isNullX(leftExpression), call));
}
Object[] list = bin.getNodeMetaData(StaticCompilationMetadataKeys.BINARY_EXP_TARGET);
@@ -188,7 +202,7 @@ public class BinaryExpressionTransformer {
// right == null ? 1 : left.compareTo(right)
Expression expr = ternaryX(
- boolX(new CompareToNullExpression(right, true)),
+ new CompareToNullExpression(right, true),
CONSTANT_ONE,
call
);
@@ -196,7 +210,7 @@ public class BinaryExpressionTransformer {
// left == null ? -1 : (right == null ? 1 : left.compareTo(right))
expr = ternaryX(
- boolX(new CompareToNullExpression(left, true)),
+ new CompareToNullExpression(left, true),
CONSTANT_MINUS_ONE,
expr
);
@@ -204,7 +218,7 @@ public class BinaryExpressionTransformer {
// left === right ? 0 : (left == null ? -1 : (right == null ? 1 : left.compareTo(right)))
expr = ternaryX(
- boolX(new CompareIdentityExpression(left, right)),
+ new CompareIdentityExpression(left, right),
CONSTANT_ZERO,
expr
);
@@ -357,19 +371,6 @@ public class BinaryExpressionTransformer {
return null;
}
- private static Expression convertInOperatorToTernary(final BinaryExpression bin, final Expression rightExpression, final Expression leftExpression) {
- MethodCallExpression call = callX(rightExpression, "isCase", leftExpression);
- call.setMethodTarget(bin.getNodeMetaData(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET));
- call.setSourcePosition(bin);
- call.copyNodeMetaData(bin);
- Expression tExp = ternaryX(
- boolX(binX(rightExpression, Token.newSymbol("==", -1, -1), nullX())),
- binX(leftExpression, Token.newSymbol("==", -1, -1), nullX()),
- call
- );
- return tExp;
- }
-
private static DeclarationExpression optimizeConstantInitialization(final BinaryExpression originalDeclaration, final Token operation, final ConstantExpression constant, final Expression leftExpression, final ClassNode declarationType) {
Expression cexp = constX(convertConstant((Number) constant.getValue(), ClassHelper.getWrapper(declarationType)), true);
cexp.setType(declarationType);