Re: [PATCH] D32914: Introduce Wzero-as-null-pointer-constant.

[Nico Weber via cfe-commits]

On Thu, Jul 13, 2017 at 1:38 PM, Roman Lebedev via Phabricator via
cfe-commits  wrote:

> lebedev.ri added a comment.
>
> PR #33771  filled.
> IMHO the problems with this diagnostic should be resolved before 5.0
>

iirc gcc warns in that case too (?) Also, the warning is off by default.


>
>
> https://reviews.llvm.org/D32914
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32914: Introduce Wzero-as-null-pointer-constant.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

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


https://reviews.llvm.org/D32914



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32914: Introduce Wzero-as-null-pointer-constant.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

@thakis ping?


https://reviews.llvm.org/D32914



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32914: Introduce Wzero-as-null-pointer-constant.

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added a comment.

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
  class TemplateClass {
   public:
explicit TemplateClass(int a, T default_value = 0) {}
  };
  void IgnoreSubstTemplateType() {
TemplateClass 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


https://reviews.llvm.org/D32914



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32914: Introduce Wzero-as-null-pointer-constant.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

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


https://reviews.llvm.org/D32914



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32914: Introduce Wzero-as-null-pointer-constant.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

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.


https://reviews.llvm.org/D32914



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32914: Introduce Wzero-as-null-pointer-constant.

[Nico Weber via Phabricator via cfe-commits]

thakis marked an inline comment as done.
thakis added a comment.

s/Add one/All done/


https://reviews.llvm.org/D32914



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32914: Introduce Wzero-as-null-pointer-constant.

[Nico Weber via Phabricator via cfe-commits]

thakis closed this revision.
thakis marked 3 inline comments as done.
thakis added a comment.

Thanks! Add one and landed in r302247.




Comment at: include/clang/Sema/Sema.h:3760
 
+  /// \brief Warn when implicitly casting 0 to nullptr.
+  void diagnoseZeroToNullptrConversion(CastKind Kind, const Expr *E);

hans wrote:
> \brief is redudant here I believe and doesn't seem used for the surrounding 
> functions.
diagnoseNullableToNonnullConversion right above has it but the rest doesn't. 
removed.


https://reviews.llvm.org/D32914



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32914: Introduce Wzero-as-null-pointer-constant.

[Hans Wennborg via Phabricator via cfe-commits]

hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.

Nice. Some comments, but lgtm.




Comment at: include/clang/Sema/Sema.h:3760
 
+  /// \brief Warn when implicitly casting 0 to nullptr.
+  void diagnoseZeroToNullptrConversion(CastKind Kind, const Expr *E);

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



Comment at: lib/Sema/Sema.cpp:396
+  Diag(E->getLocStart(), diag::warn_zero_as_null_pointer_constant)
+  << FixItHint::CreateReplacement(E->getSourceRange(), "nullptr");
+}

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



Comment at: test/SemaCXX/warn-zero-nullptr.cpp:9
+int (S::*mp1) = 0; // expected-warning{{zero as null pointer constant}}
+void* p1 = 0; // expected-warning{{zero as null pointer constant}}
+

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.


https://reviews.llvm.org/D32914



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32914: Introduce Wzero-as-null-pointer-constant.

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.

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


https://reviews.llvm.org/D32914

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/Sema.cpp
  test/SemaCXX/warn-zero-nullptr.cpp


Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -8962,6 +8962,9 @@
   "implicit conversion from nullable pointer %0 to non-nullable pointer "
   "type %1">,
   InGroup, DefaultIgnore;
+def warn_zero_as_null_pointer_constant : Warning<
+  "zero as null pointer constant">,
+  InGroup>, DefaultIgnore;
 
 def err_nullability_cs_multilevel : Error<
   "nullability keyword %0 cannot be applied to multi-level pointer type %1">;
Index: test/SemaCXX/warn-zero-nullptr.cpp
===
--- test/SemaCXX/warn-zero-nullptr.cpp
+++ test/SemaCXX/warn-zero-nullptr.cpp
@@ -0,0 +1,24 @@
+// 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(0); // expected-warning{{zero as 
null pointer constant}}
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -3757,6 +3757,9 @@
   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 ) {
 return DelayedDiagnostics.push(pool);
   }
Index: lib/Sema/Sema.cpp
===
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -383,6 +383,19 @@
   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.
@@ -407,6 +420,7 @@
 #endif
 
   diagnoseNullableToNonnullConversion(Ty, E->getType(), E->getLocStart());
+  diagnoseZeroToNullptrConversion(Kind, E);
 
   QualType ExprTy = Context.getCanonicalType(E->getType());
   QualType TypeTy = Context.getCanonicalType(Ty);


Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -8962,6 +8962,9 @@
   "implicit conversion from nullable pointer %0 to non-nullable pointer "
   "type %1">,
   InGroup, DefaultIgnore;
+def warn_zero_as_null_pointer_constant : Warning<
+  "zero as null pointer constant">,
+  InGroup>, DefaultIgnore;
 
 def err_nullability_cs_multilevel : Error<
   "nullability keyword %0 cannot be applied to multi-level pointer type %1">;
Index: test/SemaCXX/warn-zero-nullptr.cpp
===
--- test/SemaCXX/warn-zero-nullptr.cpp
+++ test/SemaCXX/warn-zero-nullptr.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -Wzero-as-null-pointer-constant -std=c++11
+
+struct S {};
+
+int