This is more a wish than a bug report, sorry if this is not the proper place to talk about this. It also contains two scenarios that for my "i know nothing about compilers" seem to be the same thing, but if you prefer me to close this and file two different issues i'm happy to also do that :) I like to have -Wzero-as-null-pointer-constant enabled, but when interfacing with C libraries it's painful because it says "you're using a 0 when you should use nullptr", but in some cases "you can't". Scenario 1: #### main.cpp #### extern "C" { #include "myzlib.h" } int main(int argc, char **argv) { void *bla = Z_NULL; } #### myzlib.h #### #define Z_NULL 0 I guess you could argue here that one could stop using Z_NULL and use nullptr directly but given that Z_NULL is what the documentation says to use, it would be cool if one could see that Z_NULL is defined as 0 inside an include wrapped by extern "C" and not give a warning Scenario 2: #### main.cpp #### extern "C" { #include "clib.h" } int main(int argc, char **argv) { simple(3); } #### clib.h #### typedef struct { int value; int *something; } MyStruct; #define simple(x) MyStruct s; s.value = x; s.something = 0; In this case the use of 0 as nullptr is totally inside the extern "C" header, so there's nothing one can do to not have a warning.
This is with gcc version 9.1.0
maybe try putting #pragma GCC system_header in your headers? idk
If the header files can be modified, there is no need to put any pragmas in there, the header files can just be modified to not use a literal 0. If the header files cannot be modified, the -isystem command-line option also causes included files to be treated as system headers where warnings are suppressed. If there is going to be some other way to suppress -Wzero-as-null-pointer-constant, it should not simply depend on extern "C". The problem here isn't that it's in an extern "C" block, it's that it's in a header you cannot change, and you propose using the presence of an extern "C" block to detect that. The problem with that is that it will also suppress the warning for extern "C" blocks that you can change. (Of course, in headers meant to be compatible with C, you cannot use nullptr, but you do not need to: you can include <stddef.h> and use NULL, which will not trigger this warning.)
(In reply to Eric Gallager from comment #2) > maybe try putting #pragma GCC system_header in your headers? idk Please no, that's for marking a header as part of "the implementation" not a diagnostic-suppression mechanism for user code!
(In reply to Harald van Dijk from comment #3) > If the header files cannot be modified, the -isystem command-line option > also causes included files to be treated as system headers where warnings > are suppressed. The thing is, this isn't happening. If you compile the attached main-glib.cpp with g++ -c -Wzero-as-null-pointer-constant main.cpp -isystem /usr/include/glib-2.0/ -isystem /usr/lib/glib-2.0/include i still get main.cpp: In function ‘GType poppler_annot_get_type()’: main.cpp:17:1: warning: zero as null pointer constant [-Wzero-as-null-pointer-constant] 17 | G_DEFINE_TYPE (PopplerAnnot, poppler_annot, G_TYPE_OBJECT) | ^~~~~~~~~~~~~
Created attachment 46609 [details] the said main-glib.cpp file well the line has to be g++ -c -Wzero-as-null-pointer-constant main-glib.cpp -isystem /usr/include/glib-2.0/ -isystem /usr/lib/glib-2.0/include and not g++ -c -Wzero-as-null-pointer-constant main.cpp -isystem /usr/include/glib-2.0/ -isystem /usr/lib/glib-2.0/include also you need to have glib2 installed somewhere, may be different paths than mine
(In reply to Albert Astals Cid from comment #5) > (In reply to Harald van Dijk from comment #3) > > If the header files cannot be modified, the -isystem command-line option > > also causes included files to be treated as system headers where warnings > > are suppressed. > > The thing is, this isn't happening. If you compile the attached > main-glib.cpp with > > g++ -c -Wzero-as-null-pointer-constant main.cpp -isystem > /usr/include/glib-2.0/ -isystem /usr/lib/glib-2.0/include Huh, indeed. It works with the Ubuntu-provided GCC 8.3.0. It does not work with the Ubuntu-provided GCC 9.1.0. $ g++-8 -v Using built-in specs. COLLECT_GCC=g++-8 COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/8/lto-wrapper OFFLOAD_TARGET_NAMES=nvptx-none OFFLOAD_TARGET_DEFAULT=1 Target: x86_64-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Ubuntu 8.3.0-6ubuntu1' --with-bugurl=file:///usr/share/doc/gcc-8/README.Bugs --enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++ --prefix=/usr --with-gcc-major-version-only --program-suffix=-8 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-libmpx --enable-plugin --enable-default-pie --with-system-zlib --with-target-system-zlib --enable-objc-gc=auto --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none --without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu Thread model: posix gcc version 8.3.0 (Ubuntu 8.3.0-6ubuntu1) $ g++-9 -v Using built-in specs. COLLECT_GCC=g++-9 COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/9/lto-wrapper OFFLOAD_TARGET_NAMES=nvptx-none OFFLOAD_TARGET_DEFAULT=1 Target: x86_64-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Ubuntu 9.1.0-2ubuntu2~19.04' --with-bugurl=file:///usr/share/doc/gcc-9/README.Bugs --enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++ --prefix=/usr --with-gcc-major-version-only --program-suffix=-9 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-plugin --enable-default-pie --with-system-zlib --with-target-system-zlib=auto --enable-objc-gc=auto --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none --without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu Thread model: posix gcc version 9.1.0 (Ubuntu 9.1.0-2ubuntu2~19.04) $ g++-8 -c -Wzero-as-null-pointer-constant main.cpp -isystem /usr/include/glib-2.0 -isystem /usr/lib/x86_64-linux-gnu/glib-2.0/include $ g++-9 -c -Wzero-as-null-pointer-constant main.cpp -isystem /usr/include/glib-2.0 -isystem /usr/lib/x86_64-linux-gnu/glib-2.0/include main.cpp: In function ‘GType poppler_annot_get_type()’: main.cpp:16:1: warning: zero as null pointer constant [-Wzero-as-null-pointer-constant] 16 | G_DEFINE_TYPE (PopplerAnnot, poppler_annot, G_TYPE_OBJECT) | ^~~~~~~~~~~~~ With GCC 9 without -isystem, it still shows the warning as coming from the header ("/usr/include/glib-2.0/glib/gthread.h:250:42: warning: zero as null pointer constant"), but when -isystem is used like that, the warning location is moved to main.cpp, so no longer suppressed. I do not know at this moment whether this is an intentional change in GCC 9.
(In reply to Albert Astals Cid from comment #0) > #define Z_NULL 0 N.B. this is actually a bug. Using Z_NULL where a null pointer is expected might cause bugs e.g. execl("foo", "foo", Z_NULL) can pass a 32-bit int where a 64-bit pointer is expected, leading to undefined behaviour. So maybe the warning should be taken as an opportunity to fix the code (or report a bug to the library) to use something safer: #if __cplusplus #define Z_NULL nullptr #else #define Z_NULL 0L #endif
(In reply to Jonathan Wakely from comment #8) > (In reply to Albert Astals Cid from comment #0) > > #define Z_NULL 0 > > N.B. this is actually a bug. Using Z_NULL where a null pointer is expected > might cause bugs e.g. execl("foo", "foo", Z_NULL) can pass a 32-bit int > where a 64-bit pointer is expected, leading to undefined behaviour. Whether this is a bug in the definition of Z_NULL depends on how Z_NULL is meant to be used. If it is not supported as a variadic function argument, the fact that it does not work as a variadic function argument is not a bug. zlib.h's definition contains a comment: #define Z_NULL 0 /* for initializing zalloc, zfree, opaque */ This does not include calling variadic functions. > So maybe the warning should be taken as an opportunity to fix the code (or > report a bug to the library) to use something safer: > > #if __cplusplus > #define Z_NULL nullptr > #else > #define Z_NULL 0L > #endif If Z_NULL is meant to be usable as a variadic function argument, this is wrong too. There is no guarantee that 0L is the same size as a pointer (think W64). There is not even a guarantee that NULL is the same size as a pointer.
(In reply to Harald van Dijk from comment #9) > (In reply to Jonathan Wakely from comment #8) > > (In reply to Albert Astals Cid from comment #0) > > > #define Z_NULL 0 > > > > N.B. this is actually a bug. Using Z_NULL where a null pointer is expected > > might cause bugs e.g. execl("foo", "foo", Z_NULL) can pass a 32-bit int > > where a 64-bit pointer is expected, leading to undefined behaviour. > > Whether this is a bug in the definition of Z_NULL depends on how Z_NULL is > meant to be used. If it is not supported as a variadic function argument, > the fact that it does not work as a variadic function argument is not a bug. > > zlib.h's definition contains a comment: > > #define Z_NULL 0 /* for initializing zalloc, zfree, opaque */ > > This does not include calling variadic functions. > > > So maybe the warning should be taken as an opportunity to fix the code (or > > report a bug to the library) to use something safer: > > > > #if __cplusplus > > #define Z_NULL nullptr > > #else > > #define Z_NULL 0L > > #endif > > If Z_NULL is meant to be usable as a variadic function argument, this is > wrong too. There is no guarantee that 0L is the same size as a pointer > (think W64). There is not even a guarantee that NULL is the same size as a > pointer. Maybe not a bug if you're sure it will never be used for variadic functions, but that is still a bad definition of Z_NULL. Why not this? #ifdef __cplusplus #define Z_NULL nullptr #else #define Z_NULL NULL #endif