You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by jw...@apache.org on 2017/10/02 00:45:21 UTC
groovy git commit: GROOVY-4018: Make the Groovy truth value of NaN be
false (closes #409)
Repository: groovy
Updated Branches:
refs/heads/master da50d1e05 -> 344c8840a
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/344c8840
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/344c8840
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/344c8840
Branch: refs/heads/master
Commit: 344c8840a567e53163465a26c2774e7d2c9f01f1
Parents: da50d1e
Author: John Wagenleitner <jw...@apache.org>
Authored: Sun Oct 1 16:28:15 2017 -0700
Committer: John Wagenleitner <jw...@apache.org>
Committed: Sun Oct 1 16:31:23 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/344c8840/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/344c8840/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/344c8840/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/344c8840/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/344c8840/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()
+ '''
+ }
+}