-Wzero-as-null-pointer-constant hits a few places

[feroldi]

Compiling with Clang 5.0.0, with libc++ and flag -Wzero-as-null-pointer-constant hits a few usages of NULL:

error: zero as null pointer constant [-Werror,-Wzero-as-null-pointer-constant]
TEST(SourceManagerTest, calcLineOffsets)
^
/usr/include/gtest/gtest.h:2187:42: note: expanded from macro 'TEST'
# define TEST(test_case_name, test_name) GTEST_TEST(test_case_name, test_name)
                                         ^
/usr/include/gtest/gtest.h:2181:3: note: expanded from macro 'GTEST_TEST'
  GTEST_TEST_(test_case_name, test_name, \
  ^
/usr/include/gtest/internal/gtest-internal.h:1228:38: note: expanded from macro 'GTEST_TEST_'
        #test_case_name, #test_name, NULL, NULL, \
                                     ^
/usr/lib/clang/5.0.0/include/stddef.h:100:18: note: expanded from macro 'NULL'
#    define NULL __null
                 ^

I presume it'd be a good choice to change from NULL to nullptr, but I'm not sure about how many other places would need to be changed as well, and whether the change is worth it.

[pavelkryukov]

There is no nullptr in C++98

[NickeZ]

Is there a temporary fix for this?

[martin-ejdestig]

I commented in the LLVM bugzilla about this:

https://bugs.llvm.org/show_bug.cgi?id=33771#c5

[barkovv]

@martin-ejdestig

https://bugs.llvm.org/show_bug.cgi?id=33771#c5

In this discussion it is said that NULL macro is still warned (with "-Wzero-as-null-pointer-constant") since it is integer zero constant.

From the README.md:

• 1.8.x Release - the 1.8.x is the last release that works with pre-C++11 compilers. The 1.8.x will not accept any requests for any new features and any bugfix requests will only be accepted if proven "critical"

So, if the c++98 support will be dropped in future releases, can we just replace all NULLs with nullptr?

[martin-ejdestig]

@barkovv I do not see your point. I think it's a Clang bug that it warns for this in "system headers" (which is also the title of the linked bug). GCC gets it right.

[gennadiycivil]

@martin-ejdestig

https://bugs.llvm.org/show_bug.cgi?id=33771#c5

In this discussion it is said that NULL macro is still warned (with "-Wzero-as-null-pointer-constant") since it is integer zero constant.

From the README.md:

• 1.8.x Release - the 1.8.x is the last release that works with pre-C++11 compilers. The 1.8.x will not accept any requests for any new features and any bugfix requests will only be accepted if proven "critical"

So, if the c++98 support will be dropped in future releases, can we just replace all NULLs with nullptr?

C++98 is already dropped in the master
Thanks

[barkovv]

@martin-ejdestig
I believe that this warning is correct. As far as I know NULL is implementation defined constant that represents a null pointer. Clang's implementation (as many others) sets it to "(void*)0".

@gennadiycivil
I can prepare PR for that.

The NULL -> nullptr migration seems to be simple (for e.g. "$ sed -i 's/NULL/nullptr/g' *.{h, cpp}"). However the manual control is required because "NULL" string can be part of function name or a comment.

[gennadiycivil]

@barkovv thank you very much, yes please prepare the PR. We have done this work in most places with clang-modernize but there are still a few left.

[martin-ejdestig]

@barkovv The point is that Clang should not warn about code the user does not control (system headers). It skips many other warnings in system headers, as does GCC, but for this particular case Clang warns even in system headers. A bit off topic so I will not respond further.

Anyway, if you fix it directly in googletest, the point is moot.

[Napoleon-BlownApart]

I've successfully done the replacement as suggested above by @barkovv.
Does the "Merged" indicate mean that the gtest package in Homebrew reflects this change?

[barkovv]

@Napoleon-BlownApart

"Merged" indicates that this issue is already fixed and the fix will be packaged in the upcoming release for your disto.