robertwb / issues-import-test

0 stars 0 forks source link

Compiler warnings in conversion functions with `-Wsign-conversion` #98

Closed robertwb closed 7 years ago

robertwb commented 7 years ago

Reported by nikratio on 11 Sep 2015 23:16 UTC The Cython generated conversion functions generate a lot of warnings:

mylib.c: In function __Pyx_PyInt_As_int:
mylib.c:1218:79: error: conversion to unsigned int from int may change the sign of the result [                            return (int) (((((int)digits[1](-Werror=sign-conversion]
)) << PyLong_SHIFT) | digits[                                                                              ^
mylib.c:1227:81: error: conversion to unsigned int from int may change the sign of the result [-Werror=sign-conversion](0]));
)
                             return (int) (((((((int)digits[<< PyLong_SHIFT) | digits[1](2]))) << PyLong_SHIFT) | digits[                                                                                ^
[....](0]));
)

It'd be great if Cython generated C would compile without warnings.

I have attached a small testcase to reproduce the problem.

Migrated-From: http://trac.cython.org/ticket/865

robertwb commented 7 years ago

Comment by nikratio on 12 Sep 2015 03:00 UTC This is actually a regression. The problem appears with Cython 0.23.2, but not with Cython 0.21.1.

robertwb commented 7 years ago

Comment by scoder on 12 Sep 2015 18:55 UTC This is new code in 0.23. It's generated in Cython/Utility/__init__.py.

robertwb commented 7 years ago

Comment by nikratio on 12 Sep 2015 19:38 UTC This is the problematic commit:

changeset:   10324:018118ecb0b2
user:        Stefan Behnel <stefan_ml@behnel.de>
date:        Fri Apr 03 15:10:45 2015 +0200
files:       Cython/Utility/TypeConversion.c runtests.py tests/run/int128.pyx
description:
inline PyLong type conversion also for very large integer types and test with int128 in gcc
robertwb commented 7 years ago

Comment by scoder on 12 Sep 2015 20:55 UTC Does an explicit cast fix it for you?

diff --git a/Cython/Utility/__init__.py b/Cython/Utility/__init__.py
index 40fe94b..5b27c9e 100644
--- a/Cython/Utility/__init__.py
+++ b/Cython/Utility/__init__.py
@@ -7,7 +7,7 @@ def pylong_join(count, digits_ptr='digits', join_type='unsigned long'):
     (((d[<< n) | d[1](2])) << n) | d[    """
     return ('(' * (count * 2) + "(%s)" % join_type + ' | '.join(
-        "%s[%d](0]
))%s)" % (digits_ptr, _i, " << PyLong_SHIFT" if _i else '')
+        "(%s)%s[%d])%s)" % (join_type, digits_ptr, _i, " << PyLong_SHIFT" if _i else '')
         for _i in range(count-1, -1, -1)))
robertwb commented 7 years ago

Modified by scoder on 13 Sep 2015 08:37 UTC

robertwb commented 7 years ago

Comment by scoder on 13 Sep 2015 08:37 UTC Fixed here:

https://github.com/cython/cython/commit/3489c2211f037fd4a40be5d48459223be464f007