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 2023/01/20 18:29:54 UTC
[groovy] branch master updated: GROOVY-10909: SC: optimize multiple nots surrounding binary expression
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 5a5726342a GROOVY-10909: SC: optimize multiple nots surrounding binary expression
5a5726342a is described below
commit 5a5726342adeb37a6fbaa8cdcbe2d47dee8dc56c
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri Jan 20 12:28:56 2023 -0600
GROOVY-10909: SC: optimize multiple nots surrounding binary expression
---
.../codehaus/groovy/ast/expr/BooleanExpression.java | 15 +++++++--------
.../org/codehaus/groovy/ast/expr/NotExpression.java | 20 +++++++++++++-------
.../transformers/BooleanExpressionTransformer.java | 12 ++++++++++++
.../sc/NaryExpressionTestStaticCompileTest.groovy | 13 +++++++++++++
4 files changed, 45 insertions(+), 15 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/ast/expr/BooleanExpression.java b/src/main/java/org/codehaus/groovy/ast/expr/BooleanExpression.java
index 744b485f2b..252fa729a2 100644
--- a/src/main/java/org/codehaus/groovy/ast/expr/BooleanExpression.java
+++ b/src/main/java/org/codehaus/groovy/ast/expr/BooleanExpression.java
@@ -27,7 +27,7 @@ import org.codehaus.groovy.ast.GroovyCodeVisitor;
public class BooleanExpression extends Expression {
private final Expression expression;
- public BooleanExpression(Expression expression) {
+ public BooleanExpression(final Expression expression) {
this.expression = expression;
setType(ClassHelper.boolean_TYPE); // for consistency with AsmClassGenerator. see AsmClassGenerator.visitBooleanExpression.
}
@@ -37,21 +37,20 @@ public class BooleanExpression extends Expression {
}
@Override
- public void visit(GroovyCodeVisitor visitor) {
- visitor.visitBooleanExpression(this);
+ public String getText() {
+ return getExpression().getText();
}
@Override
- public Expression transformExpression(ExpressionTransformer transformer) {
- Expression ret = new BooleanExpression(transformer.transform(expression));
+ public Expression transformExpression(final ExpressionTransformer transformer) {
+ Expression ret = new BooleanExpression(transformer.transform(getExpression()));
ret.setSourcePosition(this);
ret.copyNodeMetaData(this);
return ret;
}
@Override
- public String getText() {
- return expression.getText();
+ public void visit(final GroovyCodeVisitor visitor) {
+ visitor.visitBooleanExpression(this);
}
-
}
diff --git a/src/main/java/org/codehaus/groovy/ast/expr/NotExpression.java b/src/main/java/org/codehaus/groovy/ast/expr/NotExpression.java
index 6f990e59da..679f965c39 100644
--- a/src/main/java/org/codehaus/groovy/ast/expr/NotExpression.java
+++ b/src/main/java/org/codehaus/groovy/ast/expr/NotExpression.java
@@ -22,24 +22,30 @@ import org.codehaus.groovy.ast.GroovyCodeVisitor;
public class NotExpression extends BooleanExpression {
- public NotExpression(Expression expression) {
+ public NotExpression(final Expression expression) {
super(expression);
}
- @Override
- public void visit(GroovyCodeVisitor visitor) {
- visitor.visitNotExpression(this);
- }
-
+ @Deprecated
public boolean isDynamic() {
return false;
}
@Override
- public Expression transformExpression(ExpressionTransformer transformer) {
+ public String getText() {
+ return "!(" + super.getText() + ")";
+ }
+
+ @Override
+ public Expression transformExpression(final ExpressionTransformer transformer) {
Expression ret = new NotExpression(transformer.transform(getExpression()));
ret.setSourcePosition(this);
ret.copyNodeMetaData(this);
return ret;
}
+
+ @Override
+ public void visit(final GroovyCodeVisitor visitor) {
+ visitor.visitNotExpression(this);
+ }
}
diff --git a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java
index 988dfc590e..8ce3570ba6 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java
@@ -25,6 +25,7 @@ import org.codehaus.groovy.ast.MethodNode;
import org.codehaus.groovy.ast.Parameter;
import org.codehaus.groovy.ast.expr.BinaryExpression;
import org.codehaus.groovy.ast.expr.BooleanExpression;
+import org.codehaus.groovy.ast.expr.DeclarationExpression;
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.ExpressionTransformer;
import org.codehaus.groovy.ast.expr.NotExpression;
@@ -67,11 +68,22 @@ class BooleanExpressionTransformer {
if (!(expr instanceof BinaryExpression)) {
expr = transformer.transform(expr);
+
ClassNode type = transformer.getTypeChooser().resolveType(expr, transformer.getClassNode());
Expression opt = new OptimizingBooleanExpression(expr, type);
if (reverse) opt = new NotExpression(opt);
opt.setSourcePosition(boolX);
+ opt.copyNodeMetaData(boolX);
return opt;
+
+ } else if (!(expr instanceof DeclarationExpression)) {
+ // TODO: apply reverse to operation
+ expr = transformer.transform(expr);
+
+ expr = reverse ? new NotExpression(expr) : new BooleanExpression(expr);
+ expr.setSourcePosition(boolX);
+ expr.copyNodeMetaData(boolX);
+ return expr;
}
return transformer.superTransform(boolX);
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/NaryExpressionTestStaticCompileTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/NaryExpressionTestStaticCompileTest.groovy
index 50004d4c20..b4e10d09ff 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/NaryExpressionTestStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/NaryExpressionTestStaticCompileTest.groovy
@@ -39,4 +39,17 @@ class NaryExpressionTestStaticCompileTest extends STCnAryExpressionTest implemen
String out = astTrees.values()[0][1]
assert out.contains('INVOKESTATIC java/lang/Boolean.compare')
}
+
+ // GROOVY-10909
+ void testMultipleNotExpressionOptimization() {
+ assertScript '''
+ def item = "", list = []
+ def x = !!(item in list)
+ return 0
+ '''
+ String out = astTrees.values()[0][1]
+ out = out.substring(out.indexOf('run()Ljava/lang/Object;'))
+ assert !out.contains('IXOR')
+ assert !out.contains('INVOKESTATIC java/lang/Boolean.valueOf')
+ }
}