Introduce Wzero-as-null-pointer-constant.

[thakis]

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

[hans]

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.

[thakis]

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]

s/Add one/All done/

[lebedev.ri]

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.

[lebedev.ri]

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>?

[malcolm.parsons]

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

[lebedev.ri]

@thakis ping?

[lebedev.ri]

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

include/clang/Basic/DiagnosticSemaKinds.td

 
 def err_nullability_nonpointer : Error<
   "nullability specifier %0 cannot be applied to non-pointer type %1">;
 
 def warn_nullability_lost : Warning<
   "implicit conversion from nullable pointer %0 to non-nullable pointer "
   "type %1">,
   InGroup<NullableToNonNullConversion>, DefaultIgnore;
+def warn_zero_as_null_pointer_constant : Warning<
+  "zero as null pointer constant">,
+  InGroup<DiagGroup<"zero-as-null-pointer-constant">>, DefaultIgnore;
 
 def err_nullability_cs_multilevel : Error<
   "nullability keyword %0 cannot be applied to multi-level pointer type %1">;
 def note_nullability_type_specifier : Note<
   "use nullability type specifier %0 to affect the innermost "
   "pointer type of %1">;
 
 def warn_null_resettable_setter : Warning<

include/clang/Sema/Sema.h

public:
   void DiagnoseSelfMove(const Expr *LHSExpr, const Expr *RHSExpr,
                         SourceLocation OpLoc);
 
   /// \brief Warn if we're implicitly casting from a _Nullable pointer type to a
   /// _Nonnull one.
   void diagnoseNullableToNonnullConversion(QualType DstType, QualType SrcType,
                                            SourceLocation Loc);
 
+  /// \brief Warn when implicitly casting 0 to nullptr.
+  void diagnoseZeroToNullptrConversion(CastKind Kind, const Expr *E);
+
   ParsingDeclState PushParsingDeclaration(sema::DelayedDiagnosticPool &pool) {
     return DelayedDiagnostics.push(pool);
   }
   void PopParsingDeclaration(ParsingDeclState state, Decl *decl);
 
   typedef ProcessingContextState ParsingClassState;
   ParsingClassState PushParsingClass() {
     return DelayedDiagnostics.pushUndelayed();

lib/Sema/Sema.cpp

void Sema::diagnoseNullableToNonnullConversion(QualType DstType,
 
   Optional<NullabilityKind> TypeNullability = DstType->getNullability(Context);
   if (!TypeNullability || *TypeNullability != NullabilityKind::NonNull)
     return;
 
   Diag(Loc, diag::warn_nullability_lost) << SrcType << DstType;
 }
 
+void Sema::diagnoseZeroToNullptrConversion(CastKind Kind, const Expr* E) {
+  if (Kind != CK_NullToPointer && Kind != CK_NullToMemberPointer)
+    return;
+  if (E->getType()->isNullPtrType())
+    return;
+  // nullptr only exists from C++11 on, so don't warn on its absence earlier.
+  if (!getLangOpts().CPlusPlus11)
+    return;
+
+  Diag(E->getLocStart(), diag::warn_zero_as_null_pointer_constant)
+      << FixItHint::CreateReplacement(E->getSourceRange(), "nullptr");
+}
+
 /// ImpCastExprToType - If Expr is not of type 'Type', insert an implicit cast.
 /// If there is already an implicit cast, merge into the existing one.
 /// The result is of the given category.
 ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty,
                                    CastKind Kind, ExprValueKind VK,
                                    const CXXCastPath *BasePath,
                                    CheckedConversionKind CCK) {
 #ifndef NDEBUG
   if (VK == VK_RValue && !E->isRValue()) {
     switch (Kind) {
     default:
       llvm_unreachable("can't implicitly cast lvalue to rvalue with this cast "
                        "kind");
     case CK_LValueToRValue:
     case CK_ArrayToPointerDecay:
     case CK_FunctionToPointerDecay:
     case CK_ToVoid:
       break;
     }
   }
   assert((VK == VK_RValue || !E->isRValue()) && "can't cast rvalue to lvalue");
 #endif
 
   diagnoseNullableToNonnullConversion(Ty, E->getType(), E->getLocStart());
+  diagnoseZeroToNullptrConversion(Kind, E);
 
   QualType ExprTy = Context.getCanonicalType(E->getType());
   QualType TypeTy = Context.getCanonicalType(Ty);
 
   if (ExprTy == TypeTy)
     return E;
 
   // C++1z [conv.array]: The temporary materialization conversion is applied.

test/SemaCXX/warn-zero-nullptr.cpp

+// RUN: %clang_cc1 -fsyntax-only -verify %s -Wzero-as-null-pointer-constant -std=c++11
+
+struct S {};
+
+int (S::*mp0) = nullptr;
+void* p0 = nullptr;
+
+int (S::*mp1) = 0; // expected-warning{{zero as null pointer constant}}
+void* p1 = 0; // expected-warning{{zero as null pointer constant}}
+
+// NULL is an integer constant expression, so warn on it too:
+void* p2 = __null; // expected-warning{{zero as null pointer constant}}
+int (S::*mp2) = __null; // expected-warning{{zero as null pointer constant}}
+
+void f0(void* v = 0); // expected-warning{{zero as null pointer constant}}
+void f1(void* v);
+
+void g() {
+  f1(0); // expected-warning{{zero as null pointer constant}}
+}
+
+// Warn on these too. Matches gcc and arguably makes sense.
+void* pp = (decltype(nullptr))0; // expected-warning{{zero as null pointer constant}}
+void* pp2 = static_cast<decltype(nullptr)>(0); // expected-warning{{zero as null pointer constant}}