Repository: cython/cython
Pull request: Fix ctypedefed bint
Fixes #3066
Finalisation of the work in #3080



Problem statement:
Cython incorrectly casts with ctypedef and bint
Given the following code:

```
ctypedef bint mybool

cdef f():
        cdef mybool c
        cdef mybool x
        c = True
        x = True
        x = not x if c else x
        print(x)
```

Results in this C code:

```
/* "test.pyx":3
 * ctypedef bint mybool
 * 
 * cdef f():             # <<<<<<<<<<<<<<
 * 	cdef mybool c
 * 	cdef mybool x
 */

static PyObject *__pyx_f_4test_f(void) {
  __pyx_t_4test_mybool __pyx_v_c;
  __pyx_t_4test_mybool __pyx_v_x;
  PyObject *__pyx_r = NULL;
  __Pyx_RefNannyDeclarations
  PyObject *__pyx_t_1 = NULL;
  PyObject *__pyx_t_2 = NULL;
  __Pyx_RefNannySetupContext("f", 0);

  /* "test.pyx":6
 * 	cdef mybool c
 * 	cdef mybool x
 * 	c = True             # <<<<<<<<<<<<<<
 * 	x = True
 * 	x = not x if c else x
 */
  __pyx_v_c = 1;

  /* "test.pyx":7
 * 	cdef mybool x
 * 	c = True
 * 	x = True             # <<<<<<<<<<<<<<
 * 	x = not x if c else x
 * 	print(x)
 */
  __pyx_v_x = 1;

  /* "test.pyx":8
 * 	c = True
 * 	x = True
 * 	x = not x if c else x             # <<<<<<<<<<<<<<
 * 	print(x)
 */
  if ((__pyx_v_c != 0)) {
    __pyx_t_1 = ((PyObject *)(!(__pyx_v_x != 0)));
  } else {
    __pyx_t_1 = ((PyObject *)__pyx_v_x);
  }
  __pyx_v_x = ((__pyx_t_4test_mybool)__pyx_t_1);
  __pyx_t_1 = 0;

  /* "test.pyx":9
 * 	x = True
 * 	x = not x if c else x
 * 	print(x)             # <<<<<<<<<<<<<<
 */
  __pyx_t_1 = __Pyx_PyBool_FromLong(__pyx_v_x); if (unlikely(!__pyx_t_1)) __PYX_ERR(0, 9, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_1);
  __pyx_t_2 = __Pyx_PyObject_CallOneArg(__pyx_builtin_print, __pyx_t_1); if (unlikely(!__pyx_t_2)) __PYX_ERR(0, 9, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_2);
  __Pyx_DECREF(__pyx_t_1); __pyx_t_1 = 0;
  __Pyx_DECREF(__pyx_t_2); __pyx_t_2 = 0;

  /* "test.pyx":3
 * ctypedef bint mybool
 * 
 * cdef f():             # <<<<<<<<<<<<<<
 * 	cdef mybool c
 * 	cdef mybool x
 */

  /* function exit code */
  __pyx_r = Py_None; __Pyx_INCREF(Py_None);
  goto __pyx_L0;
  __pyx_L1_error:;
  __Pyx_XDECREF(__pyx_t_1);
  __Pyx_XDECREF(__pyx_t_2);
  __Pyx_AddTraceback("test.f", __pyx_clineno, __pyx_lineno, __pyx_filename);
  __pyx_r = 0;
  __pyx_L0:;
  __Pyx_XGIVEREF(__pyx_r);
  __Pyx_RefNannyFinishContext();
  return __pyx_r;
}
```

Which throws these compiler warnings:

```
$ gcc -shared -pthread -fPIC -fwrapv -O2 -Wall -fno-strict-aliasing -I/usr/include/python3.7 -o test test.c 
test.c: In function ‘__pyx_f_4test_f’:
test.c:1136:18: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
     __pyx_t_1 = ((PyObject *)(!(__pyx_v_x != 0)));
                  ^
test.c:1138:18: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
     __pyx_t_1 = ((PyObject *)__pyx_v_x);
                  ^
test.c:1140:16: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
   __pyx_v_x = ((__pyx_t_4test_mybool)__pyx_t_1);
```

The C boolean variable __pyx_v_x is incorrectly casted to PyObject * and back to bool.

This does not happen, if bint is used directly in the cdefs.

Hints from issue discussion:
Yes, that looks wrong. I guess we're too strict about special-casing `bint`, and should also include typedefs there. PR welcome that fixes up at least some of those places. Look for `c_bint_type`, mostly.
I would like to take a stab at this issue, I may need some help though @scoder.
1.  Correct me if I'm wrong, but `independent_spanning_type()` in `PyrexTypes.py` is where I should make the change? Maybe something like calling `.resolve()` on each type before comparing them to each other?
2.  When writing a test for this case, how do I check that the value is not being cast to `PyObject`? If you could point me to an existing test that does something similar, that would help a lot I think.
> how do I check that the value is not being cast to `PyObject`

You could add `# distutils: extra_compile_args=-Werror`. Doesn't look like that's done anywhere else yet, but it should make the C compilation fail if there are any warnings.

> Maybe something like calling .resolve() on each type before comparing

That doesn't sound wrong, at least. Let's see if it solves the issue at hand.

Commit message: independent_spanning_type.pyx: remove Werror

Patch to review:
diff --git a/Cython/Compiler/PyrexTypes.py b/Cython/Compiler/PyrexTypes.py
index 32ee0ad7e84..1c329fc7345 100644
--- a/Cython/Compiler/PyrexTypes.py
+++ b/Cython/Compiler/PyrexTypes.py
@@ -4929,9 +4929,10 @@ def independent_spanning_type(type1, type2):
             type1 = type1.ref_base_type
         else:
             type2 = type2.ref_base_type
-    if type1 == type2:
+    if type1.resolve() == type2.resolve():
         return type1
-    elif (type1 is c_bint_type or type2 is c_bint_type) and (type1.is_numeric and type2.is_numeric):
+    elif ((type1.resolve() is c_bint_type or type2.resolve() is c_bint_type)
+            and (type1.is_numeric and type2.is_numeric)):
         # special case: if one of the results is a bint and the other
         # is another C integer, we must prevent returning a numeric
         # type so that we do not lose the ability to coerce to a
diff --git a/tests/run/independent_spanning_type.pyx b/tests/run/independent_spanning_type.pyx
new file mode 100644
index 00000000000..7d936066444
--- /dev/null
+++ b/tests/run/independent_spanning_type.pyx
@@ -0,0 +1,69 @@
+from cython cimport typeof
+
+ctypedef bint mybool
+
+cdef mybool mybul = True
+cdef bint bul = True
+cdef int num = 42
+
+
+def CondExprNode_to_obj(test):
+    """
+    >>> CondExprNode_to_obj(True)
+    True
+    2
+    >>> CondExprNode_to_obj(False)
+    True
+    84
+    """
+
+    print(typeof(mybul if test else num) == typeof(bul if test else num) == typeof(object))
+
+    return (mybul if test else num) + (bul if test else num)
+
+
+def BoolBinopNode_to_obj():
+    """
+    >>> BoolBinopNode_to_obj()
+    True
+    2
+    """
+
+    print(typeof(mybul or num) == typeof(bul or num) == typeof(object))
+
+    return (mybul or num) + (bul or num)
+
+
+cdef int test_bool(mybool arg):
+    return <int>arg
+
+
+def CondExprNode_to_bool(test):
+    """
+    >>> CondExprNode_to_bool(True)
+    True
+    0
+    >>> CondExprNode_to_bool(False)
+    True
+    2
+    """
+
+    print(typeof(not mybul if test else mybul) == typeof(not bul if test else bul) == typeof(bul))
+
+    # test_bool would silently crash if one of the types is casted
+    # to Python object and not just assigned.
+    # It happens when type is wrongly inferred to Python object
+    # and not bin or mybool.
+    return test_bool(not mybul if test else mybul) + test_bool(not bul if test else bul)
+
+
+def BoolBinopNode_to_bool():
+    """
+    >>> BoolBinopNode_to_bool()
+    True
+    2
+    """
+
+    print(typeof(not mybul or mybul) == typeof(not bul or bul) == typeof(bul))
+
+    return test_bool(not mybul or mybul) + test_bool(not bul or bul)


Code review comments:
diff_hunk: @@ -4929,9 +4929,10 @@ def independent_spanning_type(type1, type2):
             type1 = type1.ref_base_type
         else:
             type2 = type2.ref_base_type
-    if type1 == type2:
+    if type1.resolve() == type2.resolve():
path: Cython/Compiler/PyrexTypes.py
text: @user1:
It's probably worth assigning these two to variables `resolved_type1` and `resolved_type2`.

@author:
done ae7ae936587fcc6ed79f64ba43b54edfc20c3574

diff_hunk: @@ -0,0 +1,69 @@
+from cython cimport typeof
+
+ctypedef bint mybool
+
+cdef mybool mybul = True
+cdef bint bul = True
+cdef int num = 42
+
+
+def CondExprNode_to_obj(test):
+    """
+    >>> CondExprNode_to_obj(True)
+    True
+    2
+    >>> CondExprNode_to_obj(False)
+    True
+    84
+    """
+
+    print(typeof(mybul if test else num) == typeof(bul if test else num) == typeof(object))
+
+    return (mybul if test else num) + (bul if test else num)
+
+
+def BoolBinopNode_to_obj():
+    """
+    >>> BoolBinopNode_to_obj()
+    True
+    2
+    """
+
+    print(typeof(mybul or num) == typeof(bul or num) == typeof(object))
+
+    return (mybul or num) + (bul or num)
+
+
+cdef int test_bool(mybool arg):
+    return <int>arg
+
+
+def CondExprNode_to_bool(test):
+    """
+    >>> CondExprNode_to_bool(True)
+    True
+    0
+    >>> CondExprNode_to_bool(False)
+    True
+    2
+    """
+
+    print(typeof(not mybul if test else mybul) == typeof(not bul if test else bul) == typeof(bul))
+
+    # test_bool would silently crash if one of the types is casted
+    # to Python object and not just assigned.
+    # It happens when type is wrongly inferred to Python object
+    # and not bin or mybool.
path: tests/run/independent_spanning_type.pyx
text: @user1:
```suggestion
    # test_bool() would silently crash if one of the types is cast
    # to Python object and not just assigned.
    # It happens when a type is wrongly inferred as Python object
    # instead of bint or mybool.
```