Make -Wnull-conversion more useful.

When a null constant is used in a macro, walk through the macro stack to
determine where the null constant is written and where the context is located.
Only warn if both locations are within the same macro expansion.  This helps
function-like macros which involve pointers be treated as if they were
functions.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@260776 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 44b3a18d707865f9cb716d7857f2782069a13609..7192eb501494352451332a0567272a5a70dd933e 100644 (file)
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -7265,14 +7265,21 @@ void DiagnoseNullConversion(Sema &S, Expr *E, QualType T, SourceLocation CC) {
 
   SourceLocation Loc = E->getSourceRange().getBegin();
 
+  // Venture through the macro stacks to get to the source of macro arguments.
+  // The new location is a better location than the complete location that was
+  // passed in.
+  while (S.SourceMgr.isMacroArgExpansion(Loc))
+    Loc = S.SourceMgr.getImmediateMacroCallerLoc(Loc);
+
+  while (S.SourceMgr.isMacroArgExpansion(CC))
+    CC = S.SourceMgr.getImmediateMacroCallerLoc(CC);
+
   // __null is usually wrapped in a macro.  Go up a macro if that is the case.
-  if (NullKind == Expr::NPCK_GNUNull) {
-    if (Loc.isMacroID()) {
-      StringRef MacroName = Lexer::getImmediateMacroNameForDiagnostics(
-          Loc, S.SourceMgr, S.getLangOpts());
-      if (MacroName == "NULL")
-        Loc = S.SourceMgr.getImmediateExpansionRange(Loc).first;
-    }
+  if (NullKind == Expr::NPCK_GNUNull && Loc.isMacroID()) {
+    StringRef MacroName = Lexer::getImmediateMacroNameForDiagnostics(
+        Loc, S.SourceMgr, S.getLangOpts());
+    if (MacroName == "NULL")
+      Loc = S.SourceMgr.getImmediateExpansionRange(Loc).first;
   }
 
   // Only warn if the null and context location are in the same macro expansion.

diff --git a/test/SemaCXX/conversion.cpp b/test/SemaCXX/conversion.cpp
index eea8ac2af8a5bc8aa8a4a41bb568b24697593e63..7b86cecdbcef7c36f9aed5572dd722e9e616e1dd 100644 (file)
--- a/test/SemaCXX/conversion.cpp
+++ b/test/SemaCXX/conversion.cpp
@@ -256,3 +256,45 @@ bool run() {
 }
 
 }
+
+// More tests with macros.  Specficially, test function-like macros that either
+// have a pointer return type or take pointer arguments.  Basically, if the
+// macro was changed into a function and Clang doesn't warn, then it shouldn't
+// warn for the macro either.
+namespace test13 {
+#define check_str_nullptr_13(str) ((str) ? str : nullptr)
+#define check_str_null_13(str) ((str) ? str : NULL)
+#define test13(condition) if (condition) return;
+#define identity13(arg) arg
+#define CHECK13(condition) test13(identity13(!(condition)))
+
+void function1(const char* str) {
+  CHECK13(check_str_nullptr_13(str));
+  CHECK13(check_str_null_13(str));
+}
+
+bool some_bool_function(bool);
+void function2() {
+  CHECK13(some_bool_function(nullptr));  // expected-warning{{implicit conversion of nullptr constant to 'bool'}}
+  CHECK13(some_bool_function(NULL));  // expected-warning{{implicit conversion of NULL constant to 'bool'}}
+}
+
+#define run_check_nullptr_13(str) \
+    if (check_str_nullptr_13(str)) return;
+#define run_check_null_13(str) \
+    if (check_str_null_13(str)) return;
+void function3(const char* str) {
+  run_check_nullptr_13(str)
+  run_check_null_13(str)
+  if (check_str_nullptr_13(str)) return;
+  if (check_str_null_13(str)) return;
+}
+
+void run(int* ptr);
+#define conditional_run_13(ptr) \
+    if (ptr) run(ptr);
+void function4() {
+  conditional_run_13(nullptr);
+  conditional_run_13(NULL);
+}
+}