[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: gcc-6 and sip help (bug 812138)



On 08/15/2016 12:43 PM, Gudjon I. Gudjonsson wrote:
> This fails to compile with the following message:
> 
> make[2]: Entering directory '/home/gudjon/nb/pyqwt3d/
> pyqwt3d-0.1.7~cvs20090625/build/py2.7-qt4/configure/OpenGL_Qt4'
> g++ -c -g -O2 -fstack-protector-strong -Wformat -Werror=format-security  -
> Wdate-time -D_FORTIFY_SOURCE=2 -fPIC -O2 -Wall -W -D_REENTRANT -DNDEBUG -
> DGL2PS_HAVE_ZLIB -DHAS_NUMPY -DQT_NO_DEBUG -DQT_OPENGL_LIB -I. -I/usr/include/
> qwtplot3d-qt4 -I/usr/include/python2.7 -I/usr/lib/python2.7/dist-packages/
> numpy/core/include -I/usr/include/qt4/Qt -I/usr/share/qt4/mkspecs/default -I/
> usr/include/qt4/QtOpenGL -I/usr/include/qt4 -I/usr/X11R6/include -o 
> sipOpenGLcmodule.o sipOpenGLcmodule.cpp
> sipOpenGLcmodule.cpp:4445:1: error: narrowing conversion of ‘4294967295u’ from 
> ‘unsigned int’ to ‘int’ inside { } [-Wnarrowing]
>  };

Beginning with gcc 5, when using the newest C++ standard, narrowing
conversions of constants are an error:

https://gcc.gnu.org/onlinedocs/gcc/C_002b_002b-Dialect-Options.html
   (search for "-Wnarrowing")
https://gcc.gnu.org/wiki/FAQ#Why_does_GCC_not_give_an_error_for_some_narrowing_conversions_within_list-initializations_as_required_by_C.2B-.2B-11_.28-Wnarrowing.29_.3F

Starting with gcc 6, the default C++ standard the compiler uses is
now gnu++14 (i.e. C++14 + GNU extensions). IIRC the default standard
the compiler assumed previously was still C++98. (I may be wrong
though.)

Now obviously, the problem is with the original sip file: int is
simply the wrong data type for these constants. You should use
unsigned int instead.

However, it appears that only two constants are affected here, and
the error message is actually quite misleading about the position
of the error, because sip creates a large data structure, and the
position where the error is thrown is at the end of the definition
of the data structure. If you investigate the constants that are
defined in that data structure further though, you'll realize that
only two are actually >= 0x80000000, i.e. don't fit in a signed int
anymore.

Those are:

GL_CLIENT_ALL_ATTRIB_BITS
GL_ALL_ATTRIB_BITS

which both are defined as 0xFFFFFFFF in GL/gl.h, to indicate that
all bits (that fit into 32bit int[0]) are meant.

The proper fix would probably be to replace all int constants with
unsigned int constants, but that has two issues:

 - sip doesn't appear to make a difference between int and unsigned
   int (it uses int internally), so that doesn't actually help

 - you don't want to touch the entire source file
   (ok, you could touch only those two constants, but that would
   be asymmetric)

You could probably resort to long long as a data type (long is not
sufficient on 32bit platfors), but that's really not a good idea
IMHO.

The easiest solution is probably to add -Wno-narrowing to the
compiler flags. Now due to the fact that how your pacakge build
works [1], you actually have to add 

  --extra-cxxflags="-Wno-narrowing"

to the configure.py invocation in debian/rules (for example you can
put it before --extra-libs=...). That disables the compiler
diagnostic and makes the package compile again. I just tried that
and it does build the package. [2]

However, and this was actually also a bug in the previous version
of the package, the two constants are defined "wrongly" in Python:

>>> import PyQt4.Qwt3D.OpenGL
>>> PyQt4.Qwt3D.OpenGL.GL_CLIENT_ALL_ATTRIB_BITS
-1
>>> PyQt4.Qwt3D.OpenGL.GL_ALL_ATTRIB_BITS
-1

(Should be 4294967295, if they really were unsigned.)

It's not terribly problematic, because using those constants with
a bitwise AND in python will result in the correct answer:

>>> PyQt4.Qwt3D.OpenGL.GL_ALL_ATTRIB_BITS & PyQt4.Qwt3D.OpenGL.GL_2D == PyQt4.Qwt3D.OpenGL.GL_2D
True

But a really proper fix for this would entail:

 - file a bug against sip to support unsigned int properly
 - once that's fixed, remove the -Wno-narrowing against in your
   package and use unsigned int for all of the constants, not a
   signed int (at least for those constants that are obviously
   meant to be used in a bit field context)

Hope that helps.
	
Regards,
Christian

[0] Which in theory is not completely portable, because int could
    be larger or smaller than 32 bits (though in practice it isn't
    on any arch Debian supports), the right way to define a
    constant for "all bits in an unsigned int" would be to use
    #define CONSTANT_NAME (~0u)
    However, that's probably something one should tell OpenGL
    upstream, because it's there that that constant is defined in
    that way.

[1] Btw. the entire CFLAGS = logic in debian/rules you can drop,
    because a) you're compiling C++ code, so CXXFLAGS would be
    relevant and b) the way python builds its modules is that it
    always uses the compiler and flags that python itself was
    compiled with (and it's non-trivial to override that), so
    even CXXFLAGS would likely have no effect at all.

[2] There's a reason for this diagnostic though, and I expect
    that previous build logs will have already shown that warning.
    In your case the warning can be ignored (because these type
    of "all bit" constants are rather harmless), but that's not
    always true.


Reply to: