Suppress some build warnings

[TomAugspurger]

xref pandas-dev/pandas#34114

[WillAyd]

No problem with maybe-uninitialized if it gets things through, but I think sign-compare is a pretty serious issue that can easily show up for end users so not something we should ignore

[TomAugspurger]

I agree, but I don't actually know how to do that. The wheel-building repo feels like the wrong place to ensure that we're writing clean C code.

Once I get a build passing here, I'll try to add a CI job on pandas-dev/pandas that at least compiles things with the manylinux1 docker image.

[TomAugspurger]

@WillAyd the latest failures are when compiling the C++ files

gcc -pthread -shared -Wl,--strip-debug -std=c99 -Wno-error=maybe-uninitialized -Wno-error=sign-compare -I/usr/local/include build/temp.linux-i686-3.8/pandas/_libs/testing.o -o build/lib.linux-i686-3.8/pandas/_libs/testing.cpython-38-i386-linux-gnu.so
building 'pandas._libs.window.aggregations' extension
creating build/temp.linux-i686-3.8/pandas/_libs/window
gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -std=c99 -Wno-error=maybe-uninitialized -Wno-error=sign-compare -I/usr/local/include -fPIC -DNPY_NO_DEPRECATED_API=0 -Ipandas/_libs/window -I./pandas/_libs -I/opt/python/cp38-cp38/lib/python3.8/site-packages/numpy/core/include -I/opt/python/cp38-cp38/include/python3.8 -c pandas/_libs/window/aggregations.cpp -o build/temp.linux-i686-3.8/pandas/_libs/window/aggregations.o -Werror
cc1plus: error: command line option ‘-std=c99’ is valid for C/ObjC but not for C++ [-Werror]

Because we're passing -std=c99 when I think we want -std=c++11 for the C++ files. I tried setting CPPFLAGS, but no luck

[root@1245efb813f9 pandas]# CFLAGS="-std=c99 -Wno-error=maybe-uninitialized -Wno-error=sign-compare" CPPFLAGS="-std=c++11" /opt/python/cp38-cp38/bin/python setup.py build_ext -i
running build_ext
building 'pandas._libs.window.aggregations' extension
gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -std=c99 -Wno-error=maybe-uninitialized -Wno-error=sign-compare -std=c++11 -fPIC -DNPY_NO_DEPRECATED_API=0 -Ipandas/_libs/window -I./pandas/_libs -I/opt/python/cp38-cp38/lib/python3.8/site-packages/numpy/core/include -I/opt/python/cp38-cp38/include/python3.8 -c pandas/_libs/window/aggregations.cpp -o build/temp.linux-x86_64-3.8/pandas/_libs/window/aggregations.o -Werror
cc1plus: error: command line option ‘-std=c99’ is valid for C/ObjC but not for C++ [-Werror]
cc1plus: all warnings being treated as errors
error: command 'gcc' failed with exit status 1

[TomAugspurger]

-ansi should be equivalent to -std=c99 for C mode, and -std=c++98 for C++ mode.

In errors with

pandas/_libs/algos.c:144268:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
warning: pandas/_libs/groupby.pyx:1105:26: Unreachable code

[WillAyd]

Because we're passing -std=c99 when I think we want -std=c++11 for the C++ files. I tried setting CPPFLAGS, but no luck

Hmm are you sure that was the problem? We only use the -Werror flag for gcc so I don't think that should trigger a failure from cc1plus. This warning also appears in CI for pandas but doesn't fail anything

[WillAyd]

-ansi should be equivalent to -std=c99 for C mode, and -std=c++98 for C++ mode.

Just as an FYI I think ansi is equivalent to c89 so this is probably making things more strict.

[TomAugspurger]

Ah thanks.

Hopefully pandas-dev/pandas#34409 fixes this upstream in pandas...

[WillAyd]

Is this just an attempt to reduce the binary size? Feature instead of fixing an issue?

[TomAugspurger]

Just copied from NumPy.

…

On Wed, May 27, 2020 at 10:54 AM William Ayd ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In env_vars.sh <#85 (comment)> : > @@ -0,0 +1,20 @@ +#!/usr/bin/env bash +# Environment variables for build +MACOSX_DEPLOYMENT_TARGET=10.9 +# CFLAGS override for pandas-dev/pandas#34114 +if [ "$(uname)" == "Linux" ]; then + CFLAGS="-ansi -Wno-error=maybe-uninitialized -Wno-error=sign-compare -Wno-error=return-type" +fi +# Macos's linker doesn't support stripping symbols +if [ "$(uname)" != "Darwin" ]; then Is this just an attempt to reduce the binary size? Feature instead of fixing an issue? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#85 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAKAOIQE2VBUOQXYK3MTSU3RTUZUNANCNFSM4NG64N5A> .

[TomAugspurger]

Going to try to fix all these in pandas itself. The 32-bit build is failing with a signed / unsigned-comparison.