From: David Blaikie <dblaikie@gmail.com>
Date: Wed, 16 May 2012 04:20:04 +0000 (+0000)
Subject: Include the correct conversion context locations for condition expressions.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=def07624ecc535431e0c814b4b5b842de8a06997;p=clang

Include the correct conversion context locations for condition expressions.

This improves the conversion diagnostics (by correctly pointing to the loop
construct for conversions that may've been caused by the contextual conversion
to bool caused by a condition expression) and also causes the NULL conversion
warnings to be correctly suppressed when crossing a macro boundary in such a
context. (previously, since the conversion context location was incorrect, the
suppression could not be performed)

Reported by Nico Weber as feedback to r156826.

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

diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 7a24ca0eff..98baeb0b8b 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -2402,7 +2402,10 @@ public:
   };
 
   FullExprArg MakeFullExpr(Expr *Arg) {
-    return FullExprArg(ActOnFinishFullExpr(Arg).release());
+    return MakeFullExpr(Arg, Arg ? Arg->getExprLoc() : SourceLocation());
+  }
+  FullExprArg MakeFullExpr(Expr *Arg, SourceLocation CC) {
+    return FullExprArg(ActOnFinishFullExpr(Arg, CC).release());
   }
 
   StmtResult ActOnExprStmt(FullExprArg Expr);
@@ -3795,7 +3798,11 @@ public:
   Stmt *MaybeCreateStmtWithCleanups(Stmt *SubStmt);
   ExprResult MaybeCreateExprWithCleanups(ExprResult SubExpr);
 
-  ExprResult ActOnFinishFullExpr(Expr *Expr);
+  ExprResult ActOnFinishFullExpr(Expr *Expr) {
+    return ActOnFinishFullExpr(Expr, Expr ? Expr->getExprLoc()
+                                          : SourceLocation());
+  }
+  ExprResult ActOnFinishFullExpr(Expr *Expr, SourceLocation CC);
   StmtResult ActOnFinishFullStmt(Stmt *Stmt);
 
   // Marks SS invalid if it represents an incomplete type.
diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp
index 9796ea69ba..6652beaff4 100644
--- a/lib/Parse/ParseStmt.cpp
+++ b/lib/Parse/ParseStmt.cpp
@@ -948,7 +948,7 @@ StmtResult Parser::ParseIfStatement(SourceLocation *TrailingElseLoc) {
   if (ParseParenExprOrCondition(CondExp, CondVar, IfLoc, true))
     return StmtError();
 
-  FullExprArg FullCondExp(Actions.MakeFullExpr(CondExp.get()));
+  FullExprArg FullCondExp(Actions.MakeFullExpr(CondExp.get(), IfLoc));
 
   // C99 6.8.4p3 - In C99, the body of the if statement is a scope, even if
   // there is no compound stmt.  C90 does not have this clause.  We only do this
@@ -1174,7 +1174,7 @@ StmtResult Parser::ParseWhileStatement(SourceLocation *TrailingElseLoc) {
   if (ParseParenExprOrCondition(Cond, CondVar, WhileLoc, true))
     return StmtError();
 
-  FullExprArg FullCond(Actions.MakeFullExpr(Cond.get()));
+  FullExprArg FullCond(Actions.MakeFullExpr(Cond.get(), WhileLoc));
 
   // C99 6.8.5p5 - In C99, the body of the if statement is a scope, even if
   // there is no compound stmt.  C90 does not have this clause.  We only do this
@@ -1451,7 +1451,7 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
                                                  Second.get());
       }
       SecondPartIsInvalid = Second.isInvalid();
-      SecondPart = Actions.MakeFullExpr(Second.get());
+      SecondPart = Actions.MakeFullExpr(Second.get(), ForLoc);
     }
 
     if (Tok.isNot(tok::semi)) {
diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
index c0ba2d6cf8..3400529c76 100644
--- a/lib/Sema/SemaExprCXX.cpp
+++ b/lib/Sema/SemaExprCXX.cpp
@@ -5310,7 +5310,7 @@ ExprResult Sema::IgnoredValueConversions(Expr *E) {
   return Owned(E);
 }
 
-ExprResult Sema::ActOnFinishFullExpr(Expr *FE) {
+ExprResult Sema::ActOnFinishFullExpr(Expr *FE, SourceLocation CC) {
   ExprResult FullExpr = Owned(FE);
 
   if (!FullExpr.get())
@@ -5336,7 +5336,7 @@ ExprResult Sema::ActOnFinishFullExpr(Expr *FE) {
   if (FullExpr.isInvalid())
     return ExprError();
 
-  CheckImplicitConversions(FullExpr.get(), FullExpr.get()->getExprLoc());
+  CheckImplicitConversions(FullExpr.get(), CC);
   return MaybeCreateExprWithCleanups(FullExpr);
 }
 
diff --git a/test/SemaCXX/conversion.cpp b/test/SemaCXX/conversion.cpp
index 60e440f234..a9efd8c149 100644
--- a/test/SemaCXX/conversion.cpp
+++ b/test/SemaCXX/conversion.cpp
@@ -81,6 +81,14 @@ void test3() {
   // 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
+  if (NULL_COND(true))
+    ;
+  while (NULL_COND(true))
+    ;
+  for (; NULL_COND(true); )
+    ;
+  do ;
+  while(NULL_COND(true));
 }
 
 namespace test4 {