I have to ask: why do we bother warning about 'if (NULL) {...}'? It seems obvious and valid code, with no plausible typo or mistake in it. I don't understand why we need such heavy machinery merely to suppress this *iff* the NULL comes out of some other macro... But I've not followed all of the discussion, so perhaps I'm just missing context.<br>
<br><div class="gmail_quote">On Tue, May 15, 2012 at 10:20 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: dblaikie<br>
Date: Tue May 15 23:20:04 2012<br>
New Revision: 156901<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=156901&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=156901&view=rev</a><br>
Log:<br>
Include the correct conversion context locations for condition expressions.<br>
<br>
This improves the conversion diagnostics (by correctly pointing to the loop<br>
construct for conversions that may've been caused by the contextual conversion<br>
to bool caused by a condition expression) and also causes the NULL conversion<br>
warnings to be correctly suppressed when crossing a macro boundary in such a<br>
context. (previously, since the conversion context location was incorrect, the<br>
suppression could not be performed)<br>
<br>
Reported by Nico Weber as feedback to r156826.<br>
<br>
Modified:<br>
    cfe/trunk/include/clang/Sema/Sema.h<br>
    cfe/trunk/lib/Parse/ParseStmt.cpp<br>
    cfe/trunk/lib/Sema/SemaExprCXX.cpp<br>
    cfe/trunk/test/SemaCXX/conversion.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Sema/Sema.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=156901&r1=156900&r2=156901&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=156901&r1=156900&r2=156901&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/include/clang/Sema/Sema.h (original)<br>
+++ cfe/trunk/include/clang/Sema/Sema.h Tue May 15 23:20:04 2012<br>
@@ -2402,7 +2402,10 @@<br>
   };<br>
<br>
   FullExprArg MakeFullExpr(Expr *Arg) {<br>
-    return FullExprArg(ActOnFinishFullExpr(Arg).release());<br>
+    return MakeFullExpr(Arg, Arg ? Arg->getExprLoc() : SourceLocation());<br>
+  }<br>
+  FullExprArg MakeFullExpr(Expr *Arg, SourceLocation CC) {<br>
+    return FullExprArg(ActOnFinishFullExpr(Arg, CC).release());<br>
   }<br>
<br>
   StmtResult ActOnExprStmt(FullExprArg Expr);<br>
@@ -3795,7 +3798,11 @@<br>
   Stmt *MaybeCreateStmtWithCleanups(Stmt *SubStmt);<br>
   ExprResult MaybeCreateExprWithCleanups(ExprResult SubExpr);<br>
<br>
-  ExprResult ActOnFinishFullExpr(Expr *Expr);<br>
+  ExprResult ActOnFinishFullExpr(Expr *Expr) {<br>
+    return ActOnFinishFullExpr(Expr, Expr ? Expr->getExprLoc()<br>
+                                          : SourceLocation());<br>
+  }<br>
+  ExprResult ActOnFinishFullExpr(Expr *Expr, SourceLocation CC);<br>
   StmtResult ActOnFinishFullStmt(Stmt *Stmt);<br>
<br>
   // Marks SS invalid if it represents an incomplete type.<br>
<br>
Modified: cfe/trunk/lib/Parse/ParseStmt.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=156901&r1=156900&r2=156901&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=156901&r1=156900&r2=156901&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/lib/Parse/ParseStmt.cpp (original)<br>
+++ cfe/trunk/lib/Parse/ParseStmt.cpp Tue May 15 23:20:04 2012<br>
@@ -948,7 +948,7 @@<br>
   if (ParseParenExprOrCondition(CondExp, CondVar, IfLoc, true))<br>
     return StmtError();<br>
<br>
-  FullExprArg FullCondExp(Actions.MakeFullExpr(CondExp.get()));<br>
+  FullExprArg FullCondExp(Actions.MakeFullExpr(CondExp.get(), IfLoc));<br>
<br>
   // C99 6.8.4p3 - In C99, the body of the if statement is a scope, even if<br>
   // there is no compound stmt.  C90 does not have this clause.  We only do this<br>
@@ -1174,7 +1174,7 @@<br>
   if (ParseParenExprOrCondition(Cond, CondVar, WhileLoc, true))<br>
     return StmtError();<br>
<br>
-  FullExprArg FullCond(Actions.MakeFullExpr(Cond.get()));<br>
+  FullExprArg FullCond(Actions.MakeFullExpr(Cond.get(), WhileLoc));<br>
<br>
   // C99 6.8.5p5 - In C99, the body of the if statement is a scope, even if<br>
   // there is no compound stmt.  C90 does not have this clause.  We only do this<br>
@@ -1451,7 +1451,7 @@<br>
                                                  Second.get());<br>
       }<br>
       SecondPartIsInvalid = Second.isInvalid();<br>
-      SecondPart = Actions.MakeFullExpr(Second.get());<br>
+      SecondPart = Actions.MakeFullExpr(Second.get(), ForLoc);<br>
     }<br>
<br>
     if (Tok.isNot(tok::semi)) {<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=156901&r1=156900&r2=156901&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=156901&r1=156900&r2=156901&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue May 15 23:20:04 2012<br>
@@ -5310,7 +5310,7 @@<br>
   return Owned(E);<br>
 }<br>
<br>
-ExprResult Sema::ActOnFinishFullExpr(Expr *FE) {<br>
+ExprResult Sema::ActOnFinishFullExpr(Expr *FE, SourceLocation CC) {<br>
   ExprResult FullExpr = Owned(FE);<br>
<br>
   if (!FullExpr.get())<br>
@@ -5336,7 +5336,7 @@<br>
   if (FullExpr.isInvalid())<br>
     return ExprError();<br>
<br>
-  CheckImplicitConversions(FullExpr.get(), FullExpr.get()->getExprLoc());<br>
+  CheckImplicitConversions(FullExpr.get(), CC);<br>
   return MaybeCreateExprWithCleanups(FullExpr);<br>
 }<br>
<br>
<br>
Modified: cfe/trunk/test/SemaCXX/conversion.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/conversion.cpp?rev=156901&r1=156900&r2=156901&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/conversion.cpp?rev=156901&r1=156900&r2=156901&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/test/SemaCXX/conversion.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/conversion.cpp Tue May 15 23:20:04 2012<br>
@@ -81,6 +81,14 @@<br>
   // and avoiding that helps us skip these cases:<br>
 #define NULL_COND(cond) ((cond) ? &a : NULL)<br>
   bool bl2 = NULL_COND(true); // don't warn on NULL conversion through the conditional operator across a macro boundary<br>
+  if (NULL_COND(true))<br>
+    ;<br>
+  while (NULL_COND(true))<br>
+    ;<br>
+  for (; NULL_COND(true); )<br>
+    ;<br>
+  do ;<br>
+  while(NULL_COND(true));<br>
 }<br>
<br>
 namespace test4 {<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br>