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 <[email protected]> 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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
