Page MenuHomePhabricator

Introduce Wzero-as-null-pointer-constant.
ClosedPublic

Authored by thakis on May 5 2017, 8:58 AM.

Details

Reviewers
hans
Summary

Add an opt-in warning that fires when 0 is used as a null pointer.
gcc has this warning, and there's some demand for it:
http://stackoverflow.com/questions/34953361/which-clang-warning-is-equivalent-to-wzero-as-null-pointer-constant-from-gcc
https://twitter.com/StephanTLavavej/status/859943696443166720

Diff Detail

Event Timeline

thakis created this revision.May 5 2017, 8:58 AM
hans accepted this revision.May 5 2017, 9:14 AM

Nice. Some comments, but lgtm.

include/clang/Sema/Sema.h
3760

\brief is redudant here I believe and doesn't seem used for the surrounding functions.

lib/Sema/Sema.cpp
396

I was about to say that a fixit would be nice, but you were way ahead of me :-)

test/SemaCXX/warn-zero-nullptr.cpp
9

If I understand the code correctly, the warning will fire for function pointers too because that's also CK_NullToPointer. May be worth adding to the test anyway though.

This revision is now accepted and ready to land.May 5 2017, 9:14 AM
thakis closed this revision.May 5 2017, 9:24 AM
thakis marked 3 inline comments as done.

Thanks! Add one and landed in r302247.

include/clang/Sema/Sema.h
3760

diagnoseNullableToNonnullConversion right above has it but the rest doesn't. removed.

thakis marked an inline comment as done.May 5 2017, 9:24 AM

s/Add one/All done/

This warning complains about macros from system headers, e.g. PTHREAD_MUTEX_INITIALIZER:

$ ninja -j1 -v
[1/110] /usr/bin/cmake -E __run_iwyu --tidy=/usr/local/bin/clang-tidy --source=../src/librawspeed/common/DngOpcodes.cpp -- /usr/local/bin/clang++  -DDEBUG -Isrc -I../src/librawspeed -std=c++11 -Wall -Wextra -Weverything -Wno-c++98-compat -Wno-c++98-compat-pedantic -Wno-conversion -Wno-covered-switch-default -Wno-deprecated -Wno-double-promotion -Wno-exit-time-destructors -Wno-global-constructors -Wno-gnu-zero-variadic-macro-arguments -Wno-old-style-cast -Wno-padded -Wno-sign-conversion -Wno-switch-enum -Wno-undefined-func-template -Wno-unused-macros -Wno-unused-parameter -Wno-weak-vtables -O1 -fno-optimize-sibling-calls -fsanitize=thread -fPIC   -march=native -g3 -ggdb3 -Werror -MD -MT src/librawspeed/CMakeFiles/rawspeed.dir/common/DngOpcodes.cpp.o -MF src/librawspeed/CMakeFiles/rawspeed.dir/common/DngOpcodes.cpp.o.d -o src/librawspeed/CMakeFiles/rawspeed.dir/common/DngOpcodes.cpp.o -c ../src/librawspeed/common/DngOpcodes.cpp
FAILED: src/librawspeed/CMakeFiles/rawspeed.dir/common/DngOpcodes.cpp.o 
/usr/bin/cmake -E __run_iwyu --tidy=/usr/local/bin/clang-tidy --source=../src/librawspeed/common/DngOpcodes.cpp -- /usr/local/bin/clang++  -DDEBUG -Isrc -I../src/librawspeed -std=c++11 -Wall -Wextra -Weverything -Wno-c++98-compat -Wno-c++98-compat-pedantic -Wno-conversion -Wno-covered-switch-default -Wno-deprecated -Wno-double-promotion -Wno-exit-time-destructors -Wno-global-constructors -Wno-gnu-zero-variadic-macro-arguments -Wno-old-style-cast -Wno-padded -Wno-sign-conversion -Wno-switch-enum -Wno-undefined-func-template -Wno-unused-macros -Wno-unused-parameter -Wno-weak-vtables -O1 -fno-optimize-sibling-calls -fsanitize=thread -fPIC   -march=native -g3 -ggdb3 -Werror -MD -MT src/librawspeed/CMakeFiles/rawspeed.dir/common/DngOpcodes.cpp.o -MF src/librawspeed/CMakeFiles/rawspeed.dir/common/DngOpcodes.cpp.o.d -o src/librawspeed/CMakeFiles/rawspeed.dir/common/DngOpcodes.cpp.o -c ../src/librawspeed/common/DngOpcodes.cpp
../src/librawspeed/common/Mutex.h:98:27: error: zero as null pointer constant [clang-diagnostic-zero-as-null-pointer-constant]
  pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
                          ^
/usr/include/pthread.h:87:41: note: expanded from macro 'PTHREAD_MUTEX_INITIALIZER'
  { { 0, 0, 0, 0, 0, __PTHREAD_SPINS, { 0, 0 } } }
                                        ^
In file included from ../src/librawspeed/common/DngOpcodes.cpp:25:
../src/librawspeed/common/Mutex.h:98:27: error: zero as null pointer constant [-Werror,-Wzero-as-null-pointer-constant]
  pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
                          ^
/usr/include/pthread.h:87:41: note: expanded from macro 'PTHREAD_MUTEX_INITIALIZER'
  { { 0, 0, 0, 0, 0, __PTHREAD_SPINS, { 0, 0 } } }
                                        ^
In file included from ../src/librawspeed/common/DngOpcodes.cpp:25:
../src/librawspeed/common/Mutex.h:98:27: error: zero as null pointer constant [-Werror,-Wzero-as-null-pointer-constant]
/usr/include/pthread.h:87:44: note: expanded from macro 'PTHREAD_MUTEX_INITIALIZER'
  { { 0, 0, 0, 0, 0, __PTHREAD_SPINS, { 0, 0 } } }
                                           ^
2 errors generated.
ninja: build stopped: subcommand failed.

And a lot of warnings from code using googletest, highlight:

../src/librawspeed/metadata/ColorFilterArrayTest.cpp:56:1: error: zero as null pointer constant [-Werror,-Wzero-as-null-pointer-constant]
TEST(ColorFilterArrayTestBasic, Constructor) {
^
googletest/googletest-src/googletest/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)
                                         ^
googletest/googletest-src/googletest/include/gtest/gtest.h:2181:3: note: expanded from macro 'GTEST_TEST'
  GTEST_TEST_(test_case_name, test_name, \
  ^
googletest/googletest-src/googletest/include/gtest/internal/gtest-internal.h:1228:38: note: expanded from macro 'GTEST_TEST_'
        #test_case_name, #test_name, NULL, NULL, \
                                     ^
/usr/include/clang/5.0.0/include/stddef.h:100:18: note: expanded from macro 'NULL'
#    define NULL __null
                 ^

Perhaps it should not complain about such system headers / headers included via -isystem <path>?

Have you looked at the tests for clang-tidy's modernize-use-nullptr check?

I wouldn't expect to see a warning for template types:

template<typename T>
class TemplateClass {
 public:
  explicit TemplateClass(int a, T default_value = 0) {}
};
void IgnoreSubstTemplateType() {
  TemplateClass<int*> a(1);
}
test/clang-tidy/modernize-use-nullptr.cpp:252:51: warning: zero as null pointer constant [-Wzero-as-null-pointer-constant]
  explicit TemplateClass(int a, T default_value = 0) {}
                                                  ^
                                                  nullptr

PR #33771 filled.
IMHO the problems with this diagnostic should be resolved before 5.0