From 809df34efc653c6a471f951305a88bd5e675b522 Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Thu, 26 Oct 2017 13:18:14 +0000 Subject: [PATCH] [Sema] -Wzero-as-null-pointer-constant: don't warn for system macros other than NULL. Summary: The warning was initially introduced in D32914 by @thakis, and the concerns were raised there, and later in rL302247 and PR33771. I do believe that it makes sense to relax the diagnostic e.g. in this case, when the expression originates from the system header, which can not be modified. This prevents adoption for the diagnostic for codebases which use pthreads (`PTHREAD_MUTEX_INITIALIZER`), gtest, etc. As @malcolm.parsons suggests, it *may* make sense to also not warn for the template types, but it is not obvious to me how to do that in here. Though, it still makes sense to complain about `NULL` macro. While there, add more tests. Reviewers: dblaikie, thakis, rsmith, rjmccall, aaron.ballman Reviewed By: thakis Subscribers: Rakete1111, hans, cfe-commits, thakis, malcolm.parsons Tags: #clang Differential Revision: https://reviews.llvm.org/D38954 llvm-svn: 316662 --- clang/docs/ReleaseNotes.rst | 3 + clang/lib/Sema/Sema.cpp | 16 ++++- clang/test/SemaCXX/Inputs/warn-zero-nullptr.h | 3 + clang/test/SemaCXX/warn-zero-nullptr.cpp | 64 ++++++++++++++++++- 4 files changed, 81 insertions(+), 5 deletions(-) create mode 100644 clang/test/SemaCXX/Inputs/warn-zero-nullptr.h diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 997c6d3fcdce..30afc52ad298 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -91,6 +91,9 @@ Improvements to Clang's diagnostics offset is nonzero. It also now warns about arithmetic on a null pointer treated as a cast from integer to pointer (GNU extension). +- ``-Wzero-as-null-pointer-constant`` was adjusted not to warn on null pointer + constants that originate from system macros, except ``NULL`` macro. + Non-comprehensive list of changes in this release ------------------------------------------------- diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index a9d6cb4c5851..58bca426770f 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -436,12 +436,24 @@ void Sema::diagnoseNullableToNonnullConversion(QualType DstType, } void Sema::diagnoseZeroToNullptrConversion(CastKind Kind, const Expr* E) { + if (Diags.isIgnored(diag::warn_zero_as_null_pointer_constant, + E->getLocStart())) + return; + // nullptr only exists from C++11 on, so don't warn on its absence earlier. + if (!getLangOpts().CPlusPlus11) + return; + if (Kind != CK_NullToPointer && Kind != CK_NullToMemberPointer) return; if (E->IgnoreParenImpCasts()->getType()->isNullPtrType()) return; - // nullptr only exists from C++11 on, so don't warn on its absence earlier. - if (!getLangOpts().CPlusPlus11) + + // If it is a macro from system header, and if the macro name is not "NULL", + // do not warn. + SourceLocation MaybeMacroLoc = E->getLocStart(); + if (Diags.getSuppressSystemWarnings() && + SourceMgr.isInSystemMacro(MaybeMacroLoc) && + !findMacroSpelling(MaybeMacroLoc, "NULL")) return; Diag(E->getLocStart(), diag::warn_zero_as_null_pointer_constant) diff --git a/clang/test/SemaCXX/Inputs/warn-zero-nullptr.h b/clang/test/SemaCXX/Inputs/warn-zero-nullptr.h new file mode 100644 index 000000000000..9b86c4b7b0ec --- /dev/null +++ b/clang/test/SemaCXX/Inputs/warn-zero-nullptr.h @@ -0,0 +1,3 @@ +#define NULL (0) +#define SYSTEM_MACRO (0) +#define OTHER_SYSTEM_MACRO (NULL) diff --git a/clang/test/SemaCXX/warn-zero-nullptr.cpp b/clang/test/SemaCXX/warn-zero-nullptr.cpp index 72280405d7ce..45f05fa5703b 100644 --- a/clang/test/SemaCXX/warn-zero-nullptr.cpp +++ b/clang/test/SemaCXX/warn-zero-nullptr.cpp @@ -1,4 +1,10 @@ -// RUN: %clang_cc1 -fsyntax-only -verify %s -Wzero-as-null-pointer-constant -std=c++11 +// RUN: %clang_cc1 -fsyntax-only -verify %s -isystem %S/Inputs -Wzero-as-null-pointer-constant -std=c++11 +// RUN: %clang_cc1 -fsyntax-only -verify %s -isystem %S/Inputs -DSYSTEM_WARNINGS -Wzero-as-null-pointer-constant -Wsystem-headers -std=c++11 + +#include + +#define MACRO (0) +#define MCRO(x) (x) struct S {}; @@ -15,8 +21,12 @@ void* p2 = __null; // expected-warning{{zero as null pointer constant}} void (*fp2)() = __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 f0(void* v = MACRO); // expected-warning{{zero as null pointer constant}} +void f1(void* v = NULL); // expected-warning{{zero as null pointer constant}} +void f2(void* v = MCRO(0)); // expected-warning{{zero as null pointer constant}} +void f3(void* v = MCRO(NULL)); // expected-warning{{zero as null pointer constant}} +void f4(void* v = 0); // expected-warning{{zero as null pointer constant}} +void f5(void* v); void g() { f1(0); // expected-warning{{zero as null pointer constant}} @@ -32,3 +42,51 @@ struct A { operator int*() { return nullptr; } }; void func() { if (nullptr == A()) {} } void func2() { if ((nullptr) == A()) {} } } + +template void TmplFunc0(T var) {} +void Func0Test() { + TmplFunc0(0); + TmplFunc0(0); // expected-warning {{zero as null pointer constant}} + TmplFunc0(0); // expected-warning {{zero as null pointer constant}} +} + +// FIXME: this one probably should not warn. +template void TmplFunc1(int a, T default_value = 0) {} // expected-warning{{zero as null pointer constant}} expected-warning{{zero as null pointer constant}} +void FuncTest() { + TmplFunc1(0); + TmplFunc1(0); // expected-note {{in instantiation of default function argument expression for 'TmplFunc1' required here}} + TmplFunc1(0); // expected-note {{in instantiation of default function argument expression for 'TmplFunc1' required here}} +} + +template +class TemplateClass0 { + public: + explicit TemplateClass0(T var) {} +}; +void TemplateClass0Test() { + TemplateClass0 a(0); + TemplateClass0 b(0); // expected-warning {{zero as null pointer constant}} + TemplateClass0 c(0); // expected-warning {{zero as null pointer constant}} +} + +template +class TemplateClass1 { + public: +// FIXME: this one should *NOT* warn. + explicit TemplateClass1(int a, T default_value = 0) {} // expected-warning{{zero as null pointer constant}} expected-warning{{zero as null pointer constant}} +}; +void IgnoreSubstTemplateType1() { + TemplateClass1 a(1); + TemplateClass1 b(1); // expected-note {{in instantiation of default function argument expression for 'TemplateClass1' required here}} + TemplateClass1 c(1); // expected-note {{in instantiation of default function argument expression for 'TemplateClass1' required here}} +} + +#ifndef SYSTEM_WARNINGS +// Do not warn on *any* other macros from system headers, even if they +// expand to/their expansion contains NULL. +void* sys_init = SYSTEM_MACRO; +void* sys_init2 = OTHER_SYSTEM_MACRO; +#else +void* sys_init = SYSTEM_MACRO; // expected-warning {{zero as null pointer constant}} +void* sys_init2 = OTHER_SYSTEM_MACRO; // expected-warning {{zero as null pointer constant}} +#endif