[cfe-commits] r156901 - in /cfe/trunk: include/clang/Sema/Sema.h lib/Parse/ParseStmt.cpp lib/Sema/SemaExprCXX.cpp test/SemaCXX/conversion.cpp
Nico Weber
thakis at chromium.org
Thu May 17 13:59:13 PDT 2012
On Wed, May 16, 2012 at 1:02 AM, Chandler Carruth <chandlerc at google.com>wrote:
> 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.
I asked the same thing on irc. David: Any opinion on this?
>
>
> 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
>>
>
>
> _______________________________________________
> 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/20120517/213f917d/attachment.html>
More information about the cfe-commits
mailing list