Don't warn when NULL is used within a macro but its conversion is outside a macro.

This fixes the included test case & was reported by Nico Weber.

It's a little bit nasty using the difference in the conversion context, but
seems to me like a not unreasonable solution. I did have to fix up the
conversion context for conditional operators (it seems correct to me to include
the context for which we're actually doing the comparison - across all the
nested conditionals, rather than the innermost conditional which might not
actually have the problematic implicit conversion at all) and template default
arguments (this is a bit of a hack, since we don't have the source location of
the '=' anymore, so I just used the start of the parameter - open to
suggestions there)

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

diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 1fc3c1aeff30d86105d8f75be077037f6704c970..62f45f5c916b8a6239d370ae2f9e4141d7b3f897 100644 (file)
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -4250,9 +4250,10 @@ void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
     SourceLocation Loc = E->getSourceRange().getBegin();
     if (Loc.isMacroID())
       Loc = S.SourceMgr.getImmediateExpansionRange(Loc).first;
-    S.Diag(Loc, diag::warn_impcast_null_pointer_to_integer)
-        << T << clang::SourceRange(CC)
-        << FixItHint::CreateReplacement(Loc, S.getFixItZeroLiteralForType(T));
+    if (!Loc.isMacroID() || CC.isMacroID())
+      S.Diag(Loc, diag::warn_impcast_null_pointer_to_integer)
+          << T << clang::SourceRange(CC)
+          << FixItHint::CreateReplacement(Loc, S.getFixItZeroLiteralForType(T));
     return;
   }
 
@@ -4345,14 +4346,15 @@ void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
   return;
 }
 
-void CheckConditionalOperator(Sema &S, ConditionalOperator *E, QualType T);
+void CheckConditionalOperator(Sema &S, ConditionalOperator *E,
+                              SourceLocation CC, QualType T);
 
 void CheckConditionalOperand(Sema &S, Expr *E, QualType T,
                              SourceLocation CC, bool &ICContext) {
   E = E->IgnoreParenImpCasts();
 
   if (isa<ConditionalOperator>(E))
-    return CheckConditionalOperator(S, cast<ConditionalOperator>(E), T);
+    return CheckConditionalOperator(S, cast<ConditionalOperator>(E), CC, T);
 
   AnalyzeImplicitConversions(S, E, CC);
   if (E->getType() != T)
@@ -4360,9 +4362,8 @@ void CheckConditionalOperand(Sema &S, Expr *E, QualType T,
   return;
 }
 
-void CheckConditionalOperator(Sema &S, ConditionalOperator *E, QualType T) {
-  SourceLocation CC = E->getQuestionLoc();
-
+void CheckConditionalOperator(Sema &S, ConditionalOperator *E,
+                              SourceLocation CC, QualType T) {
   AnalyzeImplicitConversions(S, E->getCond(), CC);
 
   bool Suspicious = false;
@@ -4404,7 +4405,7 @@ void AnalyzeImplicitConversions(Sema &S, Expr *OrigE, SourceLocation CC) {
   // were being fed directly into the output.
   if (isa<ConditionalOperator>(E)) {
     ConditionalOperator *CO = cast<ConditionalOperator>(E);
-    CheckConditionalOperator(S, CO, T);
+    CheckConditionalOperator(S, CO, CC, T);
     return;
   }
 

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 647b9c1fc9420191b3b30f1e749887c151070be0..e81787d5a4bb5aceba44e2b96cd76f9112a5d9c5 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -3314,7 +3314,7 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
       return ExprError();
 
     Expr *Arg = Result.takeAs<Expr>();
-    CheckImplicitConversions(Arg, Arg->getExprLoc());
+    CheckImplicitConversions(Arg, Param->getOuterLocStart());
     // Build the default argument expression.
     return Owned(CXXDefaultArgExpr::Create(Context, CallLoc, Param, Arg));
   }

diff --git a/test/SemaCXX/conversion.cpp b/test/SemaCXX/conversion.cpp
index 4b44bede8dbcb6c18d1ab67d40ad3e24b436b21f..60e440f234a894fee7efe91b2d2802c9f97811a3 100644 (file)
--- a/test/SemaCXX/conversion.cpp
+++ b/test/SemaCXX/conversion.cpp
@@ -73,13 +73,14 @@ void test3() {
   // Use FileCheck to ensure we don't get any unnecessary macro-expansion notes 
   // (that don't appear as 'real' notes & can't be seen/tested by -verify)
   // CHECK-NOT: note:
-  // CHECK: note: expanded from macro 'FNULL'
-#define FNULL NULL
-  int a2 = FNULL; // expected-warning {{implicit conversion of NULL constant to 'int'}}
-  // CHECK-NOT: note:
   // CHECK: note: expanded from macro 'FINIT'
 #define FINIT int a3 = NULL;
   FINIT // expected-warning {{implicit conversion of NULL constant to 'int'}}
+
+  // we don't catch the case of #define FOO NULL ... int i = FOO; but that seems a bit narrow anyway
+  // and avoiding that helps us skip these cases:
+#define NULL_COND(cond) ((cond) ? &a : NULL)
+  bool bl2 = NULL_COND(true); // don't warn on NULL conversion through the conditional operator across a macro boundary
 }
 
 namespace test4 {