The optimised int/float comparison was accidentally not activated when the...

The optimised int/float comparison was accidentally not activated when the result of a comparison was already known to be of type bint (instead of a Python object), nor for the "!=" operator.
These seem to be fairly common cases.

Cython/Compiler/Optimize.py

@@ -3159,21 +3159,17 @@ class OptimizeBuiltinCalls(Visitor.NodeRefCleanupMixin,
             may_return_none=True,
             utility_code=load_c_utility('py_dict_pop'))
 
-    Pyx_PyInt_BinopInt_func_type = PyrexTypes.CFuncType(
-        PyrexTypes.py_object_type, [
-            PyrexTypes.CFuncTypeArg("op1", PyrexTypes.py_object_type, None),
-            PyrexTypes.CFuncTypeArg("op2", PyrexTypes.py_object_type, None),
-            PyrexTypes.CFuncTypeArg("intval", PyrexTypes.c_long_type, None),
-            PyrexTypes.CFuncTypeArg("inplace", PyrexTypes.c_bint_type, None),
-        ])
-
-    Pyx_PyFloat_BinopInt_func_type = PyrexTypes.CFuncType(
-        PyrexTypes.py_object_type, [
-            PyrexTypes.CFuncTypeArg("op1", PyrexTypes.py_object_type, None),
-            PyrexTypes.CFuncTypeArg("op2", PyrexTypes.py_object_type, None),
-            PyrexTypes.CFuncTypeArg("fval", PyrexTypes.c_double_type, None),
-            PyrexTypes.CFuncTypeArg("inplace", PyrexTypes.c_bint_type, None),
-        ])
+    Pyx_BinopInt_func_types = dict(
+        ((ctype, ret_type), PyrexTypes.CFuncType(
+            ret_type, [
+                PyrexTypes.CFuncTypeArg("op1", PyrexTypes.py_object_type, None),
+                PyrexTypes.CFuncTypeArg("op2", PyrexTypes.py_object_type, None),
+                PyrexTypes.CFuncTypeArg("cval", ctype, None),
+                PyrexTypes.CFuncTypeArg("inplace", PyrexTypes.c_bint_type, None),
+            ], exception_value=None if ret_type.is_pyobject else ret_type.exception_value))
+        for ctype in (PyrexTypes.c_long_type, PyrexTypes.c_double_type)
+        for ret_type in (PyrexTypes.py_object_type, PyrexTypes.c_bint_type)
+        )
 
     def _handle_simple_method_object___add__(self, node, function, args, is_unbound_method):
         return self._optimise_num_binop('Add', node, function, args, is_unbound_method)
@@ -3184,7 +3180,7 @@ class OptimizeBuiltinCalls(Visitor.NodeRefCleanupMixin,
     def _handle_simple_method_object___eq__(self, node, function, args, is_unbound_method):
         return self._optimise_num_binop('Eq', node, function, args, is_unbound_method)
 
-    def _handle_simple_method_object___neq__(self, node, function, args, is_unbound_method):
+    def _handle_simple_method_object___ne__(self, node, function, args, is_unbound_method):
         return self._optimise_num_binop('Ne', node, function, args, is_unbound_method)
 
     def _handle_simple_method_object___and__(self, node, function, args, is_unbound_method):
@@ -3253,7 +3249,7 @@ class OptimizeBuiltinCalls(Visitor.NodeRefCleanupMixin,
     def _handle_simple_method_float___eq__(self, node, function, args, is_unbound_method):
         return self._optimise_num_binop('Eq', node, function, args, is_unbound_method)
 
-    def _handle_simple_method_float___neq__(self, node, function, args, is_unbound_method):
+    def _handle_simple_method_float___ne__(self, node, function, args, is_unbound_method):
         return self._optimise_num_binop('Ne', node, function, args, is_unbound_method)
 
     def _optimise_num_binop(self, operator, node, function, args, is_unbound_method):
@@ -3262,7 +3258,15 @@ class OptimizeBuiltinCalls(Visitor.NodeRefCleanupMixin,
         """
         if len(args) != 2:
             return node
-        if not node.type.is_pyobject:
+
+        if node.type.is_pyobject:
+            if operator in ('Eq', 'Ne'):
+                ret_type = PyrexTypes.c_bint_type
+            else:
+                ret_type = PyrexTypes.py_object_type
+        elif node.type is PyrexTypes.c_bint_type and operator in ('Eq', 'Ne'):
+            ret_type = PyrexTypes.c_bint_type
+        else:
             return node
 
         # When adding IntNode/FloatNode to something else, assume other operand is also numeric.
@@ -3285,6 +3289,7 @@ class OptimizeBuiltinCalls(Visitor.NodeRefCleanupMixin,
             return node
 
         is_float = isinstance(numval, ExprNodes.FloatNode)
+        num_type = PyrexTypes.c_double_type if is_float else PyrexTypes.c_long_type
         if is_float:
             if operator not in ('Add', 'Subtract', 'Remainder', 'TrueDivide', 'Divide', 'Eq', 'Ne'):
                 return node
@@ -3297,22 +3302,31 @@ class OptimizeBuiltinCalls(Visitor.NodeRefCleanupMixin,
         args = list(args)
         args.append((ExprNodes.FloatNode if is_float else ExprNodes.IntNode)(
             numval.pos, value=numval.value, constant_result=numval.constant_result,
-            type=PyrexTypes.c_double_type if is_float else PyrexTypes.c_long_type))
+            type=num_type))
         inplace = node.inplace if isinstance(node, ExprNodes.NumBinopNode) else False
         args.append(ExprNodes.BoolNode(node.pos, value=inplace, constant_result=inplace))
 
         utility_code = TempitaUtilityCode.load_cached(
             "PyFloatBinop" if is_float else "PyIntBinop", "Optimize.c",
-            context=dict(op=operator, order=arg_order))
+            context=dict(op=operator, order=arg_order, ret_type=ret_type))
 
-        return self._substitute_method_call(
-            node, function, "__Pyx_Py%s_%s%s" % ('Float' if is_float else 'Int', operator, arg_order),
-            self.Pyx_PyFloat_BinopInt_func_type if is_float else self.Pyx_PyInt_BinopInt_func_type,
+        call_node = self._substitute_method_call(
+            node, function,
+            "__Pyx_Py%s_%s%s%s" % (
+                'Float' if is_float else 'Int',
+                '' if ret_type.is_pyobject else 'Bool',
+                operator,
+                arg_order),
+            self.Pyx_BinopInt_func_types[(num_type, ret_type)],
             '__%s__' % operator[:3].lower(), is_unbound_method, args,
             may_return_none=True,
             with_none_check=False,
             utility_code=utility_code)
 
+        if node.type.is_pyobject and not ret_type.is_pyobject:
+            call_node = ExprNodes.CoerceToPyTypeNode(call_node, self.current_env(), node.type)
+        return call_node
+
     ### unicode type methods
 
     PyUnicode_uchar_predicate_func_type = PyrexTypes.CFuncType(

Cython/Utility/Optimize.c

@@ -700,11 +700,12 @@ fallback:
 
 /////////////// PyIntBinop.proto ///////////////
 
+{{py: c_ret_type = 'PyObject*' if ret_type.is_pyobject else 'int'}}
 #if !CYTHON_COMPILING_IN_PYPY
-static PyObject* __Pyx_PyInt_{{op}}{{order}}(PyObject *op1, PyObject *op2, long intval, int inplace); /*proto*/
+static {{c_ret_type}} __Pyx_PyInt_{{'' if ret_type.is_pyobject else 'Bool'}}{{op}}{{order}}(PyObject *op1, PyObject *op2, long intval, int inplace); /*proto*/
 #else
-#define __Pyx_PyInt_{{op}}{{order}}(op1, op2, intval, inplace) \
-    {{if op in ('Eq', 'Ne')}}PyObject_RichCompare(op1, op2, Py_{{op.upper()}})
+#define __Pyx_PyInt_{{'' if ret_type.is_pyobject else 'Bool'}}{{op}}{{order}}(op1, op2, intval, inplace) \
+    {{if op in ('Eq', 'Ne')}}{{'' if ret_type.is_pyobject else '__Pyx_PyObject_IsTrueAndDecref'}}(PyObject_RichCompare(op1, op2, Py_{{op.upper()}}))
     {{else}}(inplace ? PyNumber_InPlace{{op}}(op1, op2) : PyNumber_{{op}}(op1, op2))
     {{endif}}
 #endif
@@ -714,6 +715,9 @@ static PyObject* __Pyx_PyInt_{{op}}{{order}}(PyObject *op1, PyObject *op2, long
 #if !CYTHON_COMPILING_IN_PYPY
 {{py: from Cython.Utility import pylong_join }}
 {{py: pyval, ival = ('op2', 'b') if order == 'CObj' else ('op1', 'a') }}
+{{py: c_ret_type = 'PyObject*' if ret_type.is_pyobject else 'int'}}
+{{py: return_true = 'Py_RETURN_TRUE' if ret_type.is_pyobject else 'return 1'}}
+{{py: return_false = 'Py_RETURN_FALSE' if ret_type.is_pyobject else 'return 0'}}
 {{py: slot_name = {'TrueDivide': 'true_divide', 'FloorDivide': 'floor_divide'}.get(op, op.lower()) }}
 {{py:
 c_op = {
@@ -723,10 +727,10 @@ c_op = {
     }[op]
 }}
 
-static PyObject* __Pyx_PyInt_{{op}}{{order}}(PyObject *op1, PyObject *op2, CYTHON_UNUSED long intval, CYTHON_UNUSED int inplace) {
+static {{c_ret_type}} __Pyx_PyInt_{{'' if ret_type.is_pyobject else 'Bool'}}{{op}}{{order}}(PyObject *op1, PyObject *op2, CYTHON_UNUSED long intval, CYTHON_UNUSED int inplace) {
     {{if op in ('Eq', 'Ne')}}
     if (op1 == op2) {
-        Py_RETURN_{{'TRUE' if op == 'Eq' else 'FALSE'}};
+        {{return_true if op == 'Eq' else return_false}};
     }
     {{endif}}
 
@@ -740,9 +744,9 @@ static PyObject* __Pyx_PyInt_{{op}}{{order}}(PyObject *op1, PyObject *op2, CYTHO
 
         {{if op in ('Eq', 'Ne')}}
         if (a {{c_op}} b) {
-            Py_RETURN_TRUE;
+            {{return_true}};
         } else {
-            Py_RETURN_FALSE;
+            {{return_false}};
         }
         {{elif c_op in '+-'}}
             // adapted from intobject.c in Py2.7:
@@ -826,10 +830,11 @@ static PyObject* __Pyx_PyInt_{{op}}{{order}}(PyObject *op1, PyObject *op2, CYTHO
                 {{if op in ('Eq', 'Ne')}}
                 #if PyLong_SHIFT < 30 && PyLong_SHIFT != 15
                 // unusual setup - your fault
-                default: return PyLong_Type.tp_richcompare({{'op1, op2' if order == 'ObjC' else 'op2, op1'}}, Py_{{op.upper()}});
+                default: return {{'' if ret_type.is_pyobject else '__Pyx_PyObject_IsTrueAndDecref'}}(
+                    PyLong_Type.tp_richcompare({{'op1, op2' if order == 'ObjC' else 'op2, op1'}}, Py_{{op.upper()}}));
                 #else
                 // too large for the long values we allow => definitely not equal
-                default: Py_RETURN_{{'FALSE' if op == 'Eq' else 'TRUE'}};
+                default: {{return_false if op == 'Eq' else return_true}};
                 #endif
                 {{else}}
                 default: return PyLong_Type.tp_as_number->nb_{{slot_name}}(op1, op2);
@@ -838,9 +843,9 @@ static PyObject* __Pyx_PyInt_{{op}}{{order}}(PyObject *op1, PyObject *op2, CYTHO
         }
         {{if op in ('Eq', 'Ne')}}
             if (a {{c_op}} b) {
-                Py_RETURN_TRUE;
+                {{return_true}};
             } else {
-                Py_RETURN_FALSE;
+                {{return_false}};
             }
         {{else}}
             {{if c_op == '%'}}
@@ -912,9 +917,9 @@ static PyObject* __Pyx_PyInt_{{op}}{{order}}(PyObject *op1, PyObject *op2, CYTHO
         double {{ival}} = PyFloat_AS_DOUBLE({{pyval}});
         {{if op in ('Eq', 'Ne')}}
             if ((double)a {{c_op}} (double)b) {
-                Py_RETURN_TRUE;
+                {{return_true}};
             } else {
-                Py_RETURN_FALSE;
+                {{return_false}};
             }
         {{else}}
             double result;
@@ -928,7 +933,8 @@ static PyObject* __Pyx_PyInt_{{op}}{{order}}(PyObject *op1, PyObject *op2, CYTHO
     {{endif}}
 
     {{if op in ('Eq', 'Ne')}}
-    return PyObject_RichCompare(op1, op2, Py_{{op.upper()}});
+    return {{'' if ret_type.is_pyobject else '__Pyx_PyObject_IsTrueAndDecref'}}(
+        PyObject_RichCompare(op1, op2, Py_{{op.upper()}}));
     {{else}}
     return (inplace ? PyNumber_InPlace{{op}} : PyNumber_{{op}})(op1, op2);
     {{endif}}
@@ -937,11 +943,12 @@ static PyObject* __Pyx_PyInt_{{op}}{{order}}(PyObject *op1, PyObject *op2, CYTHO
 
 /////////////// PyFloatBinop.proto ///////////////
 
+{{py: c_ret_type = 'PyObject*' if ret_type.is_pyobject else 'int'}}
 #if !CYTHON_COMPILING_IN_PYPY
-static PyObject* __Pyx_PyFloat_{{op}}{{order}}(PyObject *op1, PyObject *op2, double floatval, int inplace); /*proto*/
+static {{c_ret_type}} __Pyx_PyFloat_{{'' if ret_type.is_pyobject else 'Bool'}}{{op}}{{order}}(PyObject *op1, PyObject *op2, double floatval, int inplace); /*proto*/
 #else
-#define __Pyx_PyFloat_{{op}}{{order}}(op1, op2, floatval, inplace) \
-    {{if op in ('Eq', 'Ne')}}PyObject_RichCompare(op1, op2, Py_{{op.upper()}})
+#define __Pyx_PyFloat_{{'' if ret_type.is_pyobject else 'Bool'}}{{op}}{{order}}(op1, op2, floatval, inplace) \
+    {{if op in ('Eq', 'Ne')}}{{'' if ret_type.is_pyobject else '__Pyx_PyObject_IsTrueAndDecref'}}(PyObject_RichCompare(op1, op2, Py_{{op.upper()}}))
     {{elif op == 'Divide'}}((inplace ? __Pyx_PyNumber_InPlaceDivide(op1, op2) : __Pyx_PyNumber_Divide(op1, op2)))
     {{else}}(inplace ? PyNumber_InPlace{{op}}(op1, op2) : PyNumber_{{op}}(op1, op2))
     {{endif}}
@@ -951,6 +958,9 @@ static PyObject* __Pyx_PyFloat_{{op}}{{order}}(PyObject *op1, PyObject *op2, dou
 
 #if !CYTHON_COMPILING_IN_PYPY
 {{py: from Cython.Utility import pylong_join }}
+{{py: c_ret_type = 'PyObject*' if ret_type.is_pyobject else 'int'}}
+{{py: return_true = 'Py_RETURN_TRUE' if ret_type.is_pyobject else 'return 1'}}
+{{py: return_false = 'Py_RETURN_FALSE' if ret_type.is_pyobject else 'return 0'}}
 {{py: pyval, fval = ('op2', 'b') if order == 'CObj' else ('op1', 'a') }}
 {{py:
 c_op = {
@@ -959,13 +969,13 @@ c_op = {
     }[op]
 }}
 
-static PyObject* __Pyx_PyFloat_{{op}}{{order}}(PyObject *op1, PyObject *op2, double floatval, CYTHON_UNUSED int inplace) {
+static {{c_ret_type}} __Pyx_PyFloat_{{'' if ret_type.is_pyobject else 'Bool'}}{{op}}{{order}}(PyObject *op1, PyObject *op2, double floatval, CYTHON_UNUSED int inplace) {
     const double {{'a' if order == 'CObj' else 'b'}} = floatval;
     double {{fval}}{{if op not in ('Eq', 'Ne')}}, result{{endif}};
 
     {{if op in ('Eq', 'Ne')}}
     if (op1 == op2) {
-        Py_RETURN_{{'TRUE' if op == 'Eq' else 'FALSE'}};
+        {{return_true if op == 'Eq' else return_false}};
     }
     {{endif}}
 
@@ -1011,7 +1021,8 @@ static PyObject* __Pyx_PyFloat_{{op}}{{order}}(PyObject *op1, PyObject *op2, dou
         {
         #endif
         {{if op in ('Eq', 'Ne')}}
-            return PyFloat_Type.tp_richcompare({{'op1, op2' if order == 'CObj' else 'op2, op1'}}, Py_{{op.upper()}});
+            return {{'' if ret_type.is_pyobject else '__Pyx_PyObject_IsTrueAndDecref'}}(
+                PyFloat_Type.tp_richcompare({{'op1, op2' if order == 'CObj' else 'op2, op1'}}, Py_{{op.upper()}}));
         {{else}}
             {{fval}} = PyLong_AsDouble({{pyval}});
             if (unlikely({{fval}} == -1.0 && PyErr_Occurred())) return NULL;
@@ -1019,7 +1030,8 @@ static PyObject* __Pyx_PyFloat_{{op}}{{order}}(PyObject *op1, PyObject *op2, dou
         }
     } else {
         {{if op in ('Eq', 'Ne')}}
-        return PyObject_RichCompare(op1, op2, Py_{{op.upper()}});
+        return {{'' if ret_type.is_pyobject else '__Pyx_PyObject_IsTrueAndDecref'}}(
+            PyObject_RichCompare(op1, op2, Py_{{op.upper()}}));
         {{elif op == 'Divide'}}
         return (inplace ? __Pyx_PyNumber_InPlaceDivide(op1, op2) : __Pyx_PyNumber_Divide(op1, op2));
         {{else}}
@@ -1029,9 +1041,9 @@ static PyObject* __Pyx_PyFloat_{{op}}{{order}}(PyObject *op1, PyObject *op2, dou
 
     {{if op in ('Eq', 'Ne')}}
         if (a {{c_op}} b) {
-            Py_RETURN_TRUE;
+            {{return_true}};
         } else {
-            Py_RETURN_FALSE;
+            {{return_false}};
         }
     {{else}}
         // copied from floatobject.c in Py3.5:

Cython/Utility/TypeConversion.c

@@ -86,6 +86,7 @@ static CYTHON_INLINE size_t __Pyx_Py_UNICODE_strlen(const Py_UNICODE *u) {
 #define __Pyx_Owned_Py_None(b) __Pyx_NewRef(Py_None)
 #define __Pyx_PyBool_FromLong(b) ((b) ? __Pyx_NewRef(Py_True) : __Pyx_NewRef(Py_False))
 static CYTHON_INLINE int __Pyx_PyObject_IsTrue(PyObject*);
+static CYTHON_INLINE int __Pyx_PyObject_IsTrueAndDecref(PyObject*);
 static CYTHON_INLINE PyObject* __Pyx_PyNumber_IntOrLong(PyObject* x);
 
 #define __Pyx_PySequence_Tuple(obj) \
@@ -285,6 +286,14 @@ static CYTHON_INLINE int __Pyx_PyObject_IsTrue(PyObject* x) {
    else return PyObject_IsTrue(x);
 }
 
+static CYTHON_INLINE int __Pyx_PyObject_IsTrueAndDecref(PyObject* x) {
+    int retval;
+    if (unlikely(!x)) return -1;
+    retval = __Pyx_PyObject_IsTrue(x);
+    Py_DECREF(x);
+    return retval;
+}
+
 static PyObject* __Pyx_PyNumber_IntOrLongWrongResultType(PyObject* result, const char* type_name) {
 #if PY_MAJOR_VERSION >= 3
     if (PyLong_Check(result)) {

tests/run/pyintop.pyx

 # mode: run
 
+cimport cython
+
 
 def bigint(x):
     # avoid 'L' postfix in Py2.x
@@ -10,6 +12,7 @@ def bigints(x):
     print(str(x).replace('L', ''))
 
 
+@cython.test_assert_path_exists('//IntBinopNode')
 def or_obj(obj2, obj3):
     """
     >>> or_obj(2, 3)
@@ -19,6 +22,7 @@ def or_obj(obj2, obj3):
     return obj1
 
 
+@cython.test_fail_if_path_exists('//IntBinopNode')
 def or_int(obj2):
     """
     >>> or_int(1)
@@ -30,6 +34,7 @@ def or_int(obj2):
     return obj1
 
 
+@cython.test_assert_path_exists('//IntBinopNode')
 def xor_obj(obj2, obj3):
     """
     >>> xor_obj(2, 3)
@@ -39,6 +44,7 @@ def xor_obj(obj2, obj3):
     return obj1
 
 
+@cython.test_fail_if_path_exists('//IntBinopNode')
 def xor_int(obj2):
     """
     >>> xor_int(2)
@@ -50,6 +56,7 @@ def xor_int(obj2):
     return obj1
 
 
+@cython.test_assert_path_exists('//IntBinopNode')
 def and_obj(obj2, obj3):
     """
     >>> and_obj(2, 3)
@@ -59,6 +66,7 @@ def and_obj(obj2, obj3):
     return obj1
 
 
+@cython.test_fail_if_path_exists('//IntBinopNode')
 def and_int(obj2):
     """
     >>> and_int(1)
@@ -70,6 +78,7 @@ def and_int(obj2):
     return obj1
 
 
+@cython.test_assert_path_exists('//IntBinopNode')
 def lshift_obj(obj2, obj3):
     """
     >>> lshift_obj(2, 3)
@@ -79,6 +88,7 @@ def lshift_obj(obj2, obj3):
     return obj1
 
 
+@cython.test_assert_path_exists('//IntBinopNode')
 def rshift_obj(obj2, obj3):
     """
     >>> rshift_obj(2, 3)
@@ -88,6 +98,7 @@ def rshift_obj(obj2, obj3):
     return obj1
 
 
+@cython.test_fail_if_path_exists('//IntBinopNode')
 def rshift_int(obj2):
     """
     >>> rshift_int(2)
@@ -134,6 +145,10 @@ def rshift_int(obj2):
     return obj1
 
 
+@cython.test_assert_path_exists(
+    '//SingleAssignmentNode//IntBinopNode',
+    '//SingleAssignmentNode//PythonCapiCallNode',
+)
 def lshift_int(obj):
     """
     >>> lshift_int(0)
@@ -189,6 +204,10 @@ def lshift_int(obj):
     return r1, r2, r3, r4
 
 
+@cython.test_assert_path_exists(
+    '//IntBinopNode',
+    '//IntBinopNode//IntBinopNode',
+)
 def mixed_obj(obj2, obj3):
     """
     >>> mixed_obj(2, 3)
@@ -198,6 +217,13 @@ def mixed_obj(obj2, obj3):
     return obj1
 
 
+@cython.test_assert_path_exists(
+    '//IntBinopNode',
+    '//IntBinopNode//PythonCapiCallNode',
+)
+@cython.test_fail_if_path_exists(
+    '//IntBinopNode//IntBinopNode',
+)
 def mixed_int(obj2):
     """
     >>> mixed_int(2)
@@ -209,3 +235,63 @@ def mixed_int(obj2):
     """
     obj1 = (obj2 ^ 0x10) | (obj2 & 0x01)
     return obj1
+
+
+@cython.test_assert_path_exists('//PythonCapiCallNode')
+@cython.test_fail_if_path_exists('//IntBinopNode')
+def equals(obj2):
+    """
+    >>> equals(2)
+    True
+    >>> equals(0)
+    False
+    >>> equals(-1)
+    False
+    """
+    result = obj2 == 2
+    return result
+
+
+@cython.test_assert_path_exists('//PythonCapiCallNode')
+@cython.test_fail_if_path_exists('//IntBinopNode')
+def not_equals(obj2):
+    """
+    >>> not_equals(2)
+    False
+    >>> not_equals(0)
+    True
+    >>> not_equals(-1)
+    True
+    """
+    result = obj2 != 2
+    return result
+
+
+@cython.test_assert_path_exists('//PythonCapiCallNode')
+@cython.test_fail_if_path_exists('//IntBinopNode')
+def equals_zero(obj2):
+    """
+    >>> equals_zero(2)
+    False
+    >>> equals_zero(0)
+    True
+    >>> equals_zero(-1)
+    False
+    """
+    result = obj2 == 0
+    return result
+
+
+def truthy(obj2):
+    """
+    >>> truthy(2)
+    True
+    >>> truthy(0)
+    False
+    >>> truthy(-1)
+    True
+    """
+    if obj2:
+        return True
+    else:
+        return False