groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jwagenleit...@apache.org
Subject groovy git commit: GROOVY-4018: Make the Groovy truth value of NaN be false (closes #409)
Date Mon, 02 Oct 2017 00:46:42 GMT
Repository: groovy
Updated Branches:
  refs/heads/GROOVY_2_6_X d0f1c76a3 -> 4b0123bdf


GROOVY-4018: Make the Groovy truth value of NaN be false (closes #409)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/4b0123bd
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/4b0123bd
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/4b0123bd

Branch: refs/heads/GROOVY_2_6_X
Commit: 4b0123bdfc6e9fa70b8791416d8b257c1da28312
Parents: d0f1c76
Author: John Wagenleitner <jwagenleitner@apache.org>
Authored: Sun Oct 1 16:28:15 2017 -0700
Committer: John Wagenleitner <jwagenleitner@apache.org>
Committed: Sun Oct 1 17:45:52 2017 -0700

----------------------------------------------------------------------
 .../groovy/classgen/asm/BytecodeHelper.java     | 74 ++++++++++++++++++++
 .../groovy/classgen/asm/OperandStack.java       | 37 +---------
 .../groovy/runtime/DefaultGroovyMethods.java    | 27 ++++++-
 .../BooleanExpressionTransformer.java           | 22 +-----
 src/test/groovy/bugs/Groovy4018Bug.groovy       | 68 ++++++++++++++++++
 5 files changed, 171 insertions(+), 57 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/4b0123bd/src/main/org/codehaus/groovy/classgen/asm/BytecodeHelper.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/classgen/asm/BytecodeHelper.java b/src/main/org/codehaus/groovy/classgen/asm/BytecodeHelper.java
index e227394..b9bc8e2 100644
--- a/src/main/org/codehaus/groovy/classgen/asm/BytecodeHelper.java
+++ b/src/main/org/codehaus/groovy/classgen/asm/BytecodeHelper.java
@@ -669,4 +669,78 @@ public class BytecodeHelper implements Opcodes {
         }
         return h;
     }
+
+    /**
+     * Converts a primitive type to boolean.
+     *
+     * @param mv method visitor
+     * @param type primitive type to convert
+     */
+    public static void convertPrimitiveToBoolean(MethodVisitor mv, ClassNode type) {
+        if (type == ClassHelper.boolean_TYPE) {
+            return;
+        }
+        // Special handling is done for floating point types in order to
+        // handle checking for 0 or NaN values.
+        if (type == ClassHelper.double_TYPE) {
+            convertDoubleToBoolean(mv, type);
+            return;
+        } else if (type == ClassHelper.float_TYPE) {
+            convertFloatToBoolean(mv, type);
+            return;
+        }
+        Label trueLabel = new Label();
+        Label falseLabel = new Label();
+        // Convert long to int for IFEQ comparison using LCMP
+        if (type==ClassHelper.long_TYPE) {
+            mv.visitInsn(LCONST_0);
+            mv.visitInsn(LCMP);
+        }
+        // This handles byte, short, char and int
+        mv.visitJumpInsn(IFEQ, falseLabel);
+        mv.visitInsn(ICONST_1);
+        mv.visitJumpInsn(GOTO, trueLabel);
+        mv.visitLabel(falseLabel);
+        mv.visitInsn(ICONST_0);
+        mv.visitLabel(trueLabel);
+    }
+
+    private static void convertDoubleToBoolean(MethodVisitor mv, ClassNode type) {
+        Label trueLabel = new Label();
+        Label falseLabel = new Label();
+        Label falseLabelWithPop = new Label();
+        mv.visitInsn(DUP2); // will need the extra for isNaN call if required
+        mv.visitInsn(DCONST_0);
+        mv.visitInsn(DCMPL);
+        mv.visitJumpInsn(IFEQ, falseLabelWithPop);
+        mv.visitMethodInsn(INVOKESTATIC, "java/lang/Double", "isNaN", "(D)Z", false);
+        mv.visitJumpInsn(IFNE, falseLabel);
+        mv.visitInsn(ICONST_1);
+        mv.visitJumpInsn(GOTO, trueLabel);
+        mv.visitLabel(falseLabelWithPop);
+        mv.visitInsn(POP2);
+        mv.visitLabel(falseLabel);
+        mv.visitInsn(ICONST_0);
+        mv.visitLabel(trueLabel);
+    }
+
+    private static void convertFloatToBoolean(MethodVisitor mv, ClassNode type) {
+        Label trueLabel = new Label();
+        Label falseLabel = new Label();
+        Label falseLabelWithPop = new Label();
+        mv.visitInsn(DUP); // will need the extra for isNaN call if required
+        mv.visitInsn(FCONST_0);
+        mv.visitInsn(FCMPL);
+        mv.visitJumpInsn(IFEQ, falseLabelWithPop);
+        mv.visitMethodInsn(INVOKESTATIC, "java/lang/Float", "isNaN", "(F)Z", false);
+        mv.visitJumpInsn(IFNE, falseLabel);
+        mv.visitInsn(ICONST_1);
+        mv.visitJumpInsn(GOTO, trueLabel);
+        mv.visitLabel(falseLabelWithPop);
+        mv.visitInsn(POP);
+        mv.visitLabel(falseLabel);
+        mv.visitInsn(ICONST_0);
+        mv.visitLabel(trueLabel);
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/groovy/blob/4b0123bd/src/main/org/codehaus/groovy/classgen/asm/OperandStack.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/classgen/asm/OperandStack.java b/src/main/org/codehaus/groovy/classgen/asm/OperandStack.java
index 3be622b..fa1e78b 100644
--- a/src/main/org/codehaus/groovy/classgen/asm/OperandStack.java
+++ b/src/main/org/codehaus/groovy/classgen/asm/OperandStack.java
@@ -108,7 +108,7 @@ public class OperandStack {
             if (!ClassHelper.isPrimitiveType(last)) {
                 controller.getInvocationWriter().castNonPrimitiveToBool(last);
             } else {
-                primitive2b(mv,last);
+                BytecodeHelper.convertPrimitiveToBoolean(mv, last);
             }            
         } else { 
             throw new GroovyBugError(
@@ -118,40 +118,7 @@ public class OperandStack {
         }
         stack.set(mark,ClassHelper.boolean_TYPE);
     }
-    
-    /**
-     * convert primitive (not boolean) to boolean or byte.
-     * type needs to be a primitive type (not checked) 
-     */
-    private static void primitive2b(MethodVisitor mv, ClassNode type) {
-        Label trueLabel = new Label();
-        Label falseLabel = new Label();
-        // for the various types we make first a 
-        // kind of conversion to int using a compare
-        // operation and then handle the result common
-        // for all cases. In case of long that is LCMP,
-        // for int nothing is to be done
-        if (type==ClassHelper.double_TYPE) {
-            mv.visitInsn(DCONST_0);
-            mv.visitInsn(DCMPL);
-        } else if (type==ClassHelper.long_TYPE) {
-            mv.visitInsn(LCONST_0);
-            mv.visitInsn(LCMP);
-        } else if (type==ClassHelper.float_TYPE) {
-            mv.visitInsn(FCONST_0);
-            mv.visitInsn(FCMPL);
-        } else if (type==ClassHelper.int_TYPE) {
-            // nothing, see comment above
-        }
-        mv.visitJumpInsn(IFEQ, falseLabel);
-        mv.visitInsn(ICONST_1);
-        mv.visitJumpInsn(GOTO, trueLabel);
-        mv.visitLabel(falseLabel);
-        mv.visitInsn(ICONST_0);
-        mv.visitLabel(trueLabel);
-        // other cases can be used directly
-    }
-    
+
     /**
      * remove operand stack top element using bytecode pop
      */

http://git-wip-us.apache.org/repos/asf/groovy/blob/4b0123bd/src/main/org/codehaus/groovy/runtime/DefaultGroovyMethods.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/runtime/DefaultGroovyMethods.java b/src/main/org/codehaus/groovy/runtime/DefaultGroovyMethods.java
index e77ddb2..1df7be1 100644
--- a/src/main/org/codehaus/groovy/runtime/DefaultGroovyMethods.java
+++ b/src/main/org/codehaus/groovy/runtime/DefaultGroovyMethods.java
@@ -11130,9 +11130,32 @@ public class DefaultGroovyMethods extends DefaultGroovyMethodsSupport
{
     }
 
     /**
+     * Coerce a Float instance to a boolean value.
+     *
+     * @param object the Float
+     * @return {@code true} for non-zero and non-NaN values, else {@false}
+     * @since 2.6.0
+     */
+    public static boolean asBoolean(Float object) {
+        float f = object;
+        return f != 0.0f && !Float.isNaN(f);
+    }
+
+    /**
+     * Coerce a Double instance to a boolean value.
+     *
+     * @param object the Double
+     * @return {@code true} for non-zero and non-NaN values, else {@false}
+     * @since 2.6.0
+     */
+    public static boolean asBoolean(Double object) {
+        double d = object;
+        return d != 0.0d && !Double.isNaN(d);
+    }
+
+    /**
      * Coerce a number to a boolean value.
-     * A number is coerced to false if its double value is equal to 0, and to true otherwise,
-     * and to true otherwise.
+     * A number is coerced to false if its double value is equal to 0, and to true otherwise.
      *
      * @param number the number
      * @return the boolean value

http://git-wip-us.apache.org/repos/asf/groovy/blob/4b0123bd/src/main/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java
b/src/main/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java
index e60c2cb..f0efd41 100644
--- a/src/main/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java
+++ b/src/main/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java
@@ -129,26 +129,8 @@ public class BooleanExpressionTransformer {
                     // becomes the "null" constant, so we need to recheck
                     top = controller.getOperandStack().getTopOperand();
                     if (ClassHelper.isPrimitiveType(top)) {
-                        if (top.equals(ClassHelper.int_TYPE) || top.equals(ClassHelper.byte_TYPE)
-                                || top.equals(ClassHelper.short_TYPE) || top.equals(ClassHelper.char_TYPE))
{
-                            // int on stack
-                        } else if (top.equals(ClassHelper.long_TYPE)) {
-                            MethodVisitor mv = controller.getMethodVisitor();
-                            mv.visitInsn(LCONST_0);
-                            mv.visitInsn(LCMP);
-                            controller.getOperandStack().replace(ClassHelper.boolean_TYPE);
-                        } else if (top.equals(ClassHelper.float_TYPE)) {
-                            MethodVisitor mv = controller.getMethodVisitor();
-                            mv.visitInsn(F2D);
-                            mv.visitInsn(DCONST_0);
-                            mv.visitInsn(DCMPG);
-                            controller.getOperandStack().replace(ClassHelper.boolean_TYPE);
-                        } else if (top.equals(ClassHelper.double_TYPE)) {
-                            MethodVisitor mv = controller.getMethodVisitor();
-                            mv.visitInsn(DCONST_0);
-                            mv.visitInsn(DCMPG);
-                            controller.getOperandStack().replace(ClassHelper.boolean_TYPE);
-                        }
+                        BytecodeHelper.convertPrimitiveToBoolean(controller.getMethodVisitor(),
top);
+                        controller.getOperandStack().replace(ClassHelper.boolean_TYPE);
                         return;
                     }
                 }

http://git-wip-us.apache.org/repos/asf/groovy/blob/4b0123bd/src/test/groovy/bugs/Groovy4018Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/bugs/Groovy4018Bug.groovy b/src/test/groovy/bugs/Groovy4018Bug.groovy
new file mode 100644
index 0000000..35690ce
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy4018Bug.groovy
@@ -0,0 +1,68 @@
+/*
+ *  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 groovy.bugs
+
+class Groovy4018Bug extends GroovyTestCase {
+
+    void testFloatAsBoolean() {
+        assertScript '''
+            assert 0.1f
+            assert -0.1f
+            assert Float.valueOf(0.1f)
+
+            assert !0.0f
+            assert !Float.valueOf(0.0f)
+            assert !Float.NaN
+        '''
+    }
+
+    void testDoubleAsBoolean() {
+        assertScript '''
+            assert 0.1d
+            assert -0.1d
+            assert Double.valueOf(0.1d)
+
+            assert !0.0d
+            assert !Double.valueOf(0.0d)
+            assert !Double.NaN
+        '''
+    }
+
+    void testFloatAsBooleanSTC() {
+        assertScript '''
+        @groovy.transform.CompileStatic
+        def test() {
+            assert !Float.NaN
+            assert !Float.valueOf(0.0f)
+         }
+         test()
+        '''
+    }
+
+    void testDoubleAsBooleanSTC() {
+        assertScript '''
+        @groovy.transform.CompileStatic
+        def test() {
+            assert !Double.NaN
+            assert !Double.valueOf(0.0d)
+         }
+         test()
+        '''
+    }
+}


Mime
View raw message