Index: test/SemaCXX/warn-loop-analysis.cpp =================================================================== --- test/SemaCXX/warn-loop-analysis.cpp (revision 158039) +++ test/SemaCXX/warn-loop-analysis.cpp (working copy) @@ -6,6 +6,7 @@ }; void by_ref(int &value) { } +void by_const_ref(const int &value) { } void by_value(int value) { } void by_pointer(int *value) {} @@ -18,12 +19,27 @@ for (int i; i < 1; ) { ++i; } for (int i; i < 1; ) { return; } for (int i; i < 1; ) { break; } + for (int i; i < 1; ) { goto exit_loop; } exit_loop: + for (int i; i < 1; ) { goto exit_loop; } + for (int i; i < 1; ) { by_ref(i); } + for (int i; i < 1; ) { by_const_ref(i); } // expected-warning {{variable 'i' used in loop condition not modified in loop body}} for (int i; i < 1; ) { by_value(i); } // expected-warning {{variable 'i' used in loop condition not modified in loop body}} for (int i; i < 1; ) { by_pointer(&i); } + int a, b, c, d; + by_ref(a); + by_const_ref(b); + by_value(c); + by_pointer(&d); + for (;a;); + for (;b;); // expected-warning {{variable 'b' used in loop condition not modified in loop body}} + for (;c;); // expected-warning {{variable 'c' used in loop condition not modified in loop body}} + for (;d;); + + for (int i; i < 1; ++i) for (int j; j < 1; ++j) { } @@ -42,7 +58,7 @@ } void test2() { - int i, j, k; + int i, j, k, tmp; int *ptr; // Testing CastExpr @@ -72,7 +88,7 @@ // Testing GNUNullExpr for (; ptr == __null; ) {} // expected-warning {{variable 'ptr' used in loop condition not modified in loop body}} - for (; ptr == __null; ) { ptr = &i; } + for (; ptr == __null; ) { ptr = &tmp; } // Testing UnaryOperator for (; -i > 5; ) {} // expected-warning {{variable 'i' used in loop condition not modified in loop body}} @@ -107,16 +123,13 @@ for (; i < sizeof(j); ) { ++i; } } -// False positive and how to silence. void test3() { int x; int *ptr = &x; - for (;x<5;) { *ptr = 6; } // expected-warning {{variable 'x' used in loop condition not modified in loop body}} + for (;x < 5;) { *ptr = 6; } - for (;x<5;) { - *ptr = 6; - (void)x; - } + // Uncaught infinite loop. + for (;x > 5;) { } } // Check ordering and printing of variables. Max variables is currently 4. @@ -152,3 +165,33 @@ for (;x6;); for (;y;); } + +// Testing call to functions with attribute noreturn. +extern void exit(int) __attribute__ ((__noreturn__)); +void test7() { + for (int x; x; ) + exit(x); +} + +// Testing while and do-while loops. +int foo() { return 5; } +void test8() { + int x, y, z; + while (x < y) ; // expected-warning {{variables 'x' and 'y' used in loop condition not modified in loop body}} + while (x < y) y--; + while (x++ < y) ; + do {} while (x > 1); // expected-warning {{variable 'x' used in loop condition not modified in loop body}} + do { x++;} while (x > 1); + + // Don't warn on conditional variables. + while (int i = y) ; + while (int i = foo()) ; +} + +// Don't check conditions with parameter variables. +void test9(bool run_forever) { + do { + } while(run_forever); + + while (run_forever) ; +} Index: include/clang/AST/Expr.h =================================================================== --- include/clang/AST/Expr.h (revision 158039) +++ include/clang/AST/Expr.h (working copy) @@ -1545,7 +1545,12 @@ (input->isInstantiationDependent() || type->isInstantiationDependentType()), input->containsUnexpandedParameterPack()), - Opc(opc), Loc(l), Val(input) {} + Opc(opc), Loc(l), Val(input) { + if (opc == UO_AddrOf) + if (DeclRefExpr *DRE = dyn_cast(input->IgnoreParens())) + if (VarDecl *VD = dyn_cast(DRE->getDecl())) + VD->setReference(true); + } /// \brief Build an empty unary operator. explicit UnaryOperator(EmptyShell Empty) Index: include/clang/AST/Decl.h =================================================================== --- include/clang/AST/Decl.h (revision 158039) +++ include/clang/AST/Decl.h (working copy) @@ -770,14 +770,17 @@ /// \brief Whether this variable is (C++0x) constexpr. unsigned IsConstexpr : 1; + + /// \brief Whether this variable has its address taken or is referenced. + unsigned HasReference : 1; }; - enum { NumVarDeclBits = 14 }; + enum { NumVarDeclBits = 15 }; friend class ASTDeclReader; friend class StmtIteratorBase; protected: - enum { NumParameterIndexBits = 8 }; + enum { NumParameterIndexBits = 7 }; class ParmVarDeclBitfields { friend class ParmVarDecl; @@ -1187,6 +1190,10 @@ bool isConstexpr() const { return VarDeclBits.IsConstexpr; } void setConstexpr(bool IC) { VarDeclBits.IsConstexpr = IC; } + /// Whether this variable has its address taken or is referenced. + bool hasReference() const { return VarDeclBits.HasReference; } + void setReference(bool ref) { VarDeclBits.HasReference = ref; } + /// \brief If this variable is an instantiated static data member of a /// class template specialization, returns the templated static data member /// from which it was instantiated. Index: lib/Sema/SemaInit.cpp =================================================================== --- lib/Sema/SemaInit.cpp (revision 158039) +++ lib/Sema/SemaInit.cpp (working copy) @@ -6211,6 +6211,12 @@ Expr *InitE = Init.get(); assert(InitE && "No initialization expression?"); + QualType QT = Entity.getType(); + if (QT->isReferenceType() && !QT.getNonReferenceType().isConstQualified()) + if (DeclRefExpr *DRE = dyn_cast(InitE->IgnoreParens())) + if (VarDecl *VD = dyn_cast(DRE->getDecl())) + VD->setReference(true); + if (EqualLoc.isInvalid()) EqualLoc = InitE->getLocStart(); Index: lib/Sema/SemaStmt.cpp =================================================================== --- lib/Sema/SemaStmt.cpp (revision 158039) +++ lib/Sema/SemaStmt.cpp (working copy) @@ -1061,53 +1061,6 @@ return Owned(SS); } -StmtResult -Sema::ActOnWhileStmt(SourceLocation WhileLoc, FullExprArg Cond, - Decl *CondVar, Stmt *Body) { - ExprResult CondResult(Cond.release()); - - VarDecl *ConditionVar = 0; - if (CondVar) { - ConditionVar = cast(CondVar); - CondResult = CheckConditionVariable(ConditionVar, WhileLoc, true); - if (CondResult.isInvalid()) - return StmtError(); - } - Expr *ConditionExpr = CondResult.take(); - if (!ConditionExpr) - return StmtError(); - - DiagnoseUnusedExprResult(Body); - - if (isa(Body)) - getCurCompoundScope().setHasEmptyLoopBodies(); - - return Owned(new (Context) WhileStmt(Context, ConditionVar, ConditionExpr, - Body, WhileLoc)); -} - -StmtResult -Sema::ActOnDoStmt(SourceLocation DoLoc, Stmt *Body, - SourceLocation WhileLoc, SourceLocation CondLParen, - Expr *Cond, SourceLocation CondRParen) { - assert(Cond && "ActOnDoStmt(): missing expression"); - - ExprResult CondResult = CheckBooleanCondition(Cond, DoLoc); - if (CondResult.isInvalid() || CondResult.isInvalid()) - return StmtError(); - Cond = CondResult.take(); - - CheckImplicitConversions(Cond, DoLoc); - CondResult = MaybeCreateExprWithCleanups(Cond); - if (CondResult.isInvalid()) - return StmtError(); - Cond = CondResult.take(); - - DiagnoseUnusedExprResult(Body); - - return Owned(new (Context) DoStmt(Body, Cond, DoLoc, WhileLoc, CondRParen)); -} - namespace { // This visitor will traverse a conditional statement and store all // the evaluated decls into a vector. Simple is set to true if none @@ -1196,7 +1149,6 @@ class DeclMatcher : public EvaluatedExprVisitor { llvm::SmallPtrSet &Decls; bool FoundDecl; - //bool EvalDecl; public: typedef EvaluatedExprVisitor Inherited; @@ -1220,6 +1172,13 @@ FoundDecl = true; } + void VisitCallExpr(CallExpr *E) { + if (FunctionDecl *FD = E->getDirectCallee()) + if (const FunctionType *FT = FD->getType()->getAs()) + if (FT->getNoReturnAttr()) + FoundDecl = true; + } + void VisitCastExpr(CastExpr *E) { if (E->getCastKind() == CK_LValueToRValue) CheckLValueToRValueCast(E->getSubExpr()); @@ -1262,7 +1221,7 @@ }; // end class DeclMatcher void CheckForLoopConditionalStatement(Sema &S, Expr *Second, - Expr *Third, Stmt *Body) { + Expr *Third, Stmt *Body, bool ForLoop) { // Condition is empty if (!Second) return; @@ -1286,10 +1245,15 @@ // Don't warn on volatile, static, or global variables. for (llvm::SmallPtrSet::iterator I = Decls.begin(), E = Decls.end(); - I != E; ++I) + I != E; ++I) { if ((*I)->getType().isVolatileQualified() || - (*I)->hasGlobalStorage()) return; + (*I)->hasGlobalStorage() || (*I)->hasReference()) + return; + if (!ForLoop && isa(*I)) + return; + } + if (DeclMatcher(S, Decls, Second).FoundDeclInUse() || DeclMatcher(S, Decls, Third).FoundDeclInUse() || DeclMatcher(S, Decls, Body).FoundDeclInUse()) @@ -1322,6 +1286,59 @@ } // end namespace StmtResult +Sema::ActOnWhileStmt(SourceLocation WhileLoc, FullExprArg Cond, + Decl *CondVar, Stmt *Body) { + ExprResult CondResult(Cond.release()); + + VarDecl *ConditionVar = 0; + if (CondVar) { + ConditionVar = cast(CondVar); + CondResult = CheckConditionVariable(ConditionVar, WhileLoc, true); + if (CondResult.isInvalid()) + return StmtError(); + } + Expr *ConditionExpr = CondResult.take(); + if (!ConditionExpr) + return StmtError(); + + if (!CondVar) + CheckForLoopConditionalStatement(*this, ConditionExpr, 0, Body, + /*ForLoop*/ false); + + DiagnoseUnusedExprResult(Body); + + if (isa(Body)) + getCurCompoundScope().setHasEmptyLoopBodies(); + + return Owned(new (Context) WhileStmt(Context, ConditionVar, ConditionExpr, + Body, WhileLoc)); +} + +StmtResult +Sema::ActOnDoStmt(SourceLocation DoLoc, Stmt *Body, + SourceLocation WhileLoc, SourceLocation CondLParen, + Expr *Cond, SourceLocation CondRParen) { + assert(Cond && "ActOnDoStmt(): missing expression"); + + ExprResult CondResult = CheckBooleanCondition(Cond, DoLoc); + if (CondResult.isInvalid() || CondResult.isInvalid()) + return StmtError(); + Cond = CondResult.take(); + + CheckImplicitConversions(Cond, DoLoc); + CondResult = MaybeCreateExprWithCleanups(Cond); + if (CondResult.isInvalid()) + return StmtError(); + Cond = CondResult.take(); + + CheckForLoopConditionalStatement(*this, Cond, 0, Body, /*ForLoop*/ false); + + DiagnoseUnusedExprResult(Body); + + return Owned(new (Context) DoStmt(Body, Cond, DoLoc, WhileLoc, CondRParen)); +} + +StmtResult Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc, Stmt *First, FullExprArg second, Decl *secondVar, FullExprArg third, @@ -1343,8 +1360,6 @@ } } - CheckForLoopConditionalStatement(*this, second.get(), third.get(), Body); - ExprResult SecondResult(second.release()); VarDecl *ConditionVar = 0; if (secondVar) { @@ -1354,6 +1369,10 @@ return StmtError(); } + if (!ConditionVar) + CheckForLoopConditionalStatement(*this, second.get(), third.get(), Body, + /*ForLoop*/ true); + Expr *Third = third.release().takeAs(); DiagnoseUnusedExprResult(First);