From b29394d6604c518b0239d4f382e9898e7400c7e5 Mon Sep 17 00:00:00 2001
From: Richard Trieu <rtrieu@google.com>
Date: Sat, 13 Feb 2016 00:58:53 +0000
Subject: [PATCH] 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
---
 lib/Sema/SemaChecking.cpp   | 21 ++++++++++++-------
 test/SemaCXX/conversion.cpp | 42 +++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 7 deletions(-)

diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 44b3a18d70..7192eb5014 100644
--- 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 eea8ac2af8..7b86cecdbc 100644
--- 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);
+}
+}
-- 
2.40.0