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/24 17:02:44 UTC

[groovy] branch master updated: minor items

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 ae85c50d1e minor items
ae85c50d1e is described below

commit ae85c50d1ed74bdbb407bbb19eb3b6982fbd77e4
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Jan 24 10:19:08 2023 -0600

    minor items
---
 .../groovy/ast/expr/ElvisOperatorExpression.java   | 41 +++++++++++-----------
 .../groovy/classgen/asm/StatementWriter.java       | 35 +++++++++---------
 2 files changed, 36 insertions(+), 40 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/ast/expr/ElvisOperatorExpression.java b/src/main/java/org/codehaus/groovy/ast/expr/ElvisOperatorExpression.java
index 3744840439..ced53ccf7b 100644
--- a/src/main/java/org/codehaus/groovy/ast/expr/ElvisOperatorExpression.java
+++ b/src/main/java/org/codehaus/groovy/ast/expr/ElvisOperatorExpression.java
@@ -21,43 +21,37 @@ package org.codehaus.groovy.ast.expr;
 import org.codehaus.groovy.ast.GroovyCodeVisitor;
 
 /**
- * Represents a short ternary expression x ?: y, which is equal 
- * to 
+ * Represents a short ternary expression x ?: y, which is equal to
  * <pre>
  * def truePart = x
  * def booleanPart = truePart as boolean
  * booleanPart? truePart : y
  * </pre>
- * Even if x is no atomic expression, x will be evaluated only 
- * once. Example:
+ * Even if x is no atomic expression, x will be evaluated only once. Example:
  * <pre class="groovyTestCase">
- * class Foo { 
- *   def index=0 
+ * class Foo {
+ *   def index=0
  *   def getX(){ index++; return index }
  * }
  * def foo = new Foo()
- * def result = foo.x ?: "false case" 
+ * def result = foo.x ?: "false case"
  * assert foo.index == 1
- * assert result == 1 
+ * assert result == 1
  * </pre>
- * 
+ *
  * @since 1.5
  */
 public class ElvisOperatorExpression extends TernaryExpression {
 
-    public ElvisOperatorExpression(Expression base, Expression falseExpression) {
-        super(getBool(base), base, falseExpression);
-    }
-
-    private static BooleanExpression getBool(Expression base) {
-       BooleanExpression be = new BooleanExpression(base);
-       be.setSourcePosition(base);
-       return be;
+    public ElvisOperatorExpression(final Expression base, final Expression falseValue) {
+        super(asBooleanExpression(base), base, falseValue);
     }
 
-    @Override
-    public void visit(GroovyCodeVisitor visitor) {
-        visitor.visitShortTernaryExpression(this);
+    private static BooleanExpression asBooleanExpression(final Expression base) {
+        if (base instanceof BooleanExpression) return (BooleanExpression) base;
+        BooleanExpression be = new BooleanExpression(base);
+        be.setSourcePosition(base);
+        return be;
     }
 
     @Override
@@ -67,6 +61,11 @@ public class ElvisOperatorExpression extends TernaryExpression {
                 transformer.transform(getFalseExpression()));
         ret.setSourcePosition(this);
         ret.copyNodeMetaData(this);
-        return ret; 
+        return ret;
+    }
+
+    @Override
+    public void visit(GroovyCodeVisitor visitor) {
+        visitor.visitShortTernaryExpression(this);
     }
 }
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
index f7974b6af1..0c7836fde4 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
@@ -26,6 +26,7 @@ import org.codehaus.groovy.ast.expr.ConstantExpression;
 import org.codehaus.groovy.ast.expr.EmptyExpression;
 import org.codehaus.groovy.ast.expr.Expression;
 import org.codehaus.groovy.ast.expr.MethodCallExpression;
+import org.codehaus.groovy.ast.expr.NotExpression;
 import org.codehaus.groovy.ast.stmt.AssertStatement;
 import org.codehaus.groovy.ast.stmt.BlockStatement;
 import org.codehaus.groovy.ast.stmt.BreakStatement;
@@ -230,21 +231,19 @@ public class StatementWriter {
     }
 
     private void visitConditionOfLoopingStatement(final BooleanExpression expression, final Label breakLabel, final MethodVisitor mv) {
-        boolean boolHandled = false;
-        if (expression.getExpression() instanceof ConstantExpression) {
+        if (expression.getExpression() instanceof ConstantExpression && !(expression instanceof NotExpression)) {
             ConstantExpression constant = (ConstantExpression) expression.getExpression();
-            if (constant.getValue() == Boolean.TRUE) {
-                boolHandled = true;
-                // do nothing
-            } else if (constant.getValue() == Boolean.FALSE) {
-                boolHandled = true;
+            if (constant.isFalseExpression()) {
                 mv.visitJumpInsn(GOTO, breakLabel);
+                return;
+            } else if (constant.isTrueExpression()) {
+                // do nothing
+                return;
             }
         }
-        if (!boolHandled) {
-            expression.visit(controller.getAcg());
-            controller.getOperandStack().jump(IFEQ, breakLabel);
-        }
+
+        expression.visit(controller.getAcg());
+        controller.getOperandStack().jump(IFEQ, breakLabel);
     }
 
     public void writeWhileLoop(final WhileStatement statement) {
@@ -292,19 +291,17 @@ public class StatementWriter {
         writeStatementLabel(statement);
 
         statement.getBooleanExpression().visit(controller.getAcg());
-        Label l0 = controller.getOperandStack().jump(IFEQ);
+        Label elsePath = controller.getOperandStack().jump(IFEQ);
         statement.getIfBlock().visit(controller.getAcg());
-
         MethodVisitor mv = controller.getMethodVisitor();
         if (statement.getElseBlock().isEmpty()) {
-            mv.visitLabel(l0);
+            mv.visitLabel(elsePath);
         } else {
-            Label l1 = new Label();
-            mv.visitJumpInsn(GOTO, l1);
-            mv.visitLabel(l0);
-
+            Label exitPath = new Label();
+            mv.visitJumpInsn(GOTO, exitPath);
+            mv.visitLabel(elsePath);
             statement.getElseBlock().visit(controller.getAcg());
-            mv.visitLabel(l1);
+            mv.visitLabel(exitPath);
         }
     }