[cfe-commits] r156901 - in /cfe/trunk: include/clang/Sema/Sema.h lib/Parse/ParseStmt.cpp lib/Sema/SemaExprCXX.cpp test/SemaCXX/conversion.cpp

[Chandler Carruth]

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.

On Tue, May 15, 2012 at 10:20 PM, David Blaikie <dblaikie at gmail.com> wrote:

> Author: dblaikie
> Date: Tue May 15 23:20:04 2012
> New Revision: 156901
>
> URL: http://llvm.org/viewvc/llvm-project?rev=156901&view=rev
> Log:
> 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.
>
> Modified:
>    cfe/trunk/include/clang/Sema/Sema.h
>    cfe/trunk/lib/Parse/ParseStmt.cpp
>    cfe/trunk/lib/Sema/SemaExprCXX.cpp
>    cfe/trunk/test/SemaCXX/conversion.cpp
>
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=156901&r1=156900&r2=156901&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Tue May 15 23:20:04 2012
> @@ -2402,7 +2402,10 @@
>   };
>
>   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 @@
>   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.
>
> Modified: cfe/trunk/lib/Parse/ParseStmt.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=156901&r1=156900&r2=156901&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseStmt.cpp Tue May 15 23:20:04 2012
> @@ -948,7 +948,7 @@
>   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 @@
>   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 @@
>                                                  Second.get());
>       }
>       SecondPartIsInvalid = Second.isInvalid();
> -      SecondPart = Actions.MakeFullExpr(Second.get());
> +      SecondPart = Actions.MakeFullExpr(Second.get(), ForLoc);
>     }
>
>     if (Tok.isNot(tok::semi)) {
>
> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=156901&r1=156900&r2=156901&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue May 15 23:20:04 2012
> @@ -5310,7 +5310,7 @@
>   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 @@
>   if (FullExpr.isInvalid())
>     return ExprError();
>
> -  CheckImplicitConversions(FullExpr.get(), FullExpr.get()->getExprLoc());
> +  CheckImplicitConversions(FullExpr.get(), CC);
>   return MaybeCreateExprWithCleanups(FullExpr);
>  }
>
>
> Modified: cfe/trunk/test/SemaCXX/conversion.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/conversion.cpp?rev=156901&r1=156900&r2=156901&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/conversion.cpp (original)
> +++ cfe/trunk/test/SemaCXX/conversion.cpp Tue May 15 23:20:04 2012
> @@ -81,6 +81,14 @@
>   // 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 {
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120516/bc7af1b9/attachment.html>