Add a new warning to -Wloop-analysis to warn on questionable
increments/decrements in for loop bodies. This warning triggers when the last
statement in the loop is the same as the third expression in the for-loop
header and there is no continue statements in the loop body.
Idea for this warning came from PR15636
http://llvm.org/bugs/show_bug.cgi?id=15636
http://llvm-reviews.chandlerc.com/D628
Files:
test/SemaCXX/warn-loop-analysis.cpp
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaStmt.cpp
Index: test/SemaCXX/warn-loop-analysis.cpp
===================================================================
--- test/SemaCXX/warn-loop-analysis.cpp
+++ test/SemaCXX/warn-loop-analysis.cpp
@@ -152,3 +152,111 @@
for (;x6;);
for (;y;);
}
+
+void test7() {
+ int i;
+ for (;;i++) { // expected-note{{incremented here}}
+ if (true) test7();
+ i++; // expected-warning{{both incremented}}
+ }
+ for (;;i++) { // expected-note{{incremented here}}
+ if (true) break;
+ ++i; // expected-warning{{both incremented}}
+ }
+ for (;;++i) { // expected-note{{incremented here}}
+ while (true) return;
+ i++; // expected-warning{{both incremented}}
+ }
+ for (;;++i) { // expected-note{{incremented here}}
+ ++i; // expected-warning{{both incremented}}
+ }
+
+ for (;;i--) { // expected-note{{decremented here}}
+ if (true) test7();
+ i--; // expected-warning{{both decremented}}
+ }
+ for (;;i--) { // expected-note{{decremented here}}
+ if (true) break;
+ --i; // expected-warning{{both decremented}}
+ }
+ for (;;--i) { // expected-note{{decremented here}}
+ while (true) return;
+ i--; // expected-warning{{both decremented}}
+ }
+ for (;;--i) { // expected-note{{decremented here}}
+ --i; // expected-warning{{both decremented}}
+ }
+
+ // Don't warn when loop is only one statement.
+ for (;;++i)
+ i++;
+ for (;;--i)
+ --i;
+
+ // Don't warn when loop has continue statement.
+ for (;;i++) {
+ if (true) continue;
+ i++;
+ }
+ for (;;i--) {
+ if (true) continue;
+ i--;
+ }
+}
+
+struct iterator {
+ iterator operator++() { return *this; }
+ iterator operator++(int) { return *this; }
+ iterator operator--() { return *this; }
+ iterator operator--(int) { return *this; }
+};
+void test8() {
+ iterator i;
+ for (;;i++) { // expected-note{{incremented here}}
+ if (true) test7();
+ i++; // expected-warning{{both incremented}}
+ }
+ for (;;i++) { // expected-note{{incremented here}}
+ if (true) break;
+ ++i; // expected-warning{{both incremented}}
+ }
+ for (;;++i) { // expected-note{{incremented here}}
+ while (true) return;
+ i++; // expected-warning{{both incremented}}
+ }
+ for (;;++i) { // expected-note{{incremented here}}
+ ++i; // expected-warning{{both incremented}}
+ }
+
+ for (;;i--) { // expected-note{{decremented here}}
+ if (true) test7();
+ i--; // expected-warning{{both decremented}}
+ }
+ for (;;i--) { // expected-note{{decremented here}}
+ if (true) break;
+ --i; // expected-warning{{both decremented}}
+ }
+ for (;;--i) { // expected-note{{decremented here}}
+ while (true) return;
+ i--; // expected-warning{{both decremented}}
+ }
+ for (;;--i) { // expected-note{{decremented here}}
+ --i; // expected-warning{{both decremented}}
+ }
+
+ // Don't warn when loop is only one statement.
+ for (;;++i)
+ i++;
+ for (;;--i)
+ --i;
+
+ // Don't warn when loop has continue statement.
+ for (;;i++) {
+ if (true) continue;
+ i++;
+ }
+ for (;;i--) {
+ if (true) continue;
+ i--;
+ }
+}
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -141,6 +141,7 @@
def LiteralRange : DiagGroup<"literal-range">;
def LocalTypeTemplateArgs : DiagGroup<"local-type-template-args",
[CXX98CompatLocalTypeTemplateArgs]>;
+def LoopAnalysis : DiagGroup<"loop-analysis">;
def MalformedWarningCheck : DiagGroup<"malformed-warning-check">;
def Main : DiagGroup<"main">;
def MainReturnType : DiagGroup<"main-return-type">;
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -18,7 +18,12 @@
def warn_variables_not_in_loop_body : Warning<
"variable%select{s| %1|s %1 and %2|s %1, %2, and %3|s %1, %2, %3, and %4}0 "
"used in loop condition not modified in loop body">,
- InGroup<DiagGroup<"loop-analysis">>, DefaultIgnore;
+ InGroup<LoopAnalysis>, DefaultIgnore;
+def warn_redundant_loop_iteration : Warning<
+ "variable %0 is both %select{decremented|incremented}1 in the loop header "
+ "and in the loop body">,
+ InGroup<LoopAnalysis>, DefaultIgnore;
+def note_loop_iteration_here : Note<"%select{decremented|incremented}0 here">;
def warn_duplicate_enum_values : Warning<
"element %0 has been implicitly assigned %1 which another element has "
Index: lib/Sema/SemaStmt.cpp
===================================================================
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -1404,6 +1404,104 @@
S.Diag(Ranges.begin()->getBegin(), PDiag);
}
+ // If Statement is an incemement or decrement, return true and sets the
+ // variables Increment and DRE.
+ bool ProcessIterationStmt(Sema &S, Stmt* Statement, bool &Increment,
+ DeclRefExpr *&DRE) {
+ if (UnaryOperator *UO = dyn_cast<UnaryOperator>(Statement)) {
+ switch (UO->getOpcode()) {
+ default: return false;
+ case UO_PostInc:
+ case UO_PreInc:
+ Increment = true;
+ break;
+ case UO_PostDec:
+ case UO_PreDec:
+ Increment = false;
+ break;
+ }
+ DRE = dyn_cast<DeclRefExpr>(UO->getSubExpr());
+ return DRE;
+ }
+
+ if (CXXOperatorCallExpr *Call = dyn_cast<CXXOperatorCallExpr>(Statement)) {
+ FunctionDecl *FD = Call->getDirectCallee();
+ if (!FD || !FD->isOverloadedOperator()) return false;
+ switch (FD->getOverloadedOperator()) {
+ default: return false;
+ case OO_PlusPlus:
+ Increment = true;
+ break;
+ case OO_MinusMinus:
+ Increment = false;
+ break;
+ }
+ DRE = dyn_cast<DeclRefExpr>(Call->getArg(0));
+ return DRE;
+ }
+
+ return false;
+ }
+
+ // A visitor to determine if a continue statement is a subexpression.
+ class ContinueFinder : public EvaluatedExprVisitor<ContinueFinder> {
+ bool Found;
+public:
+ ContinueFinder(Sema &S, Stmt* Body) :
+ Inherited(S.Context),
+ Found(false) {
+ Visit(Body);
+ }
+
+ typedef EvaluatedExprVisitor<ContinueFinder> Inherited;
+
+ void VisitContinueStmt(ContinueStmt* E) {
+ Found = true;
+ }
+
+ bool ContinueFound() { return Found; }
+
+ }; // end class ContinueFinder
+
+ // Emit a warning when a loop increment/decrement appears twice per loop
+ // iteration. The conditions which trigger this warning are:
+ // 1) The last statement in the loop body and the third expression in the
+ // for loop are both increment or both decrement of the same variable
+ // 2) No continue statements in the loop body.
+ void CheckForRedundantIteration(Sema &S, Expr *Third, Stmt *Body) {
+ // Return when there is nothing to check.
+ if (!Body || !Third) return;
+
+ if (S.Diags.getDiagnosticLevel(diag::warn_redundant_loop_iteration,
+ Third->getLocStart())
+ == DiagnosticsEngine::Ignored)
+ return;
+
+ // Get the last statement from the loop body.
+ CompoundStmt *CS = dyn_cast<CompoundStmt>(Body);
+ if (!CS || CS->body_empty()) return;
+ Stmt *LastStmt = CS->body_back();
+ if (!LastStmt) return;
+
+ bool LoopIncrement, LastIncrement;
+ DeclRefExpr *LoopDRE, *LastDRE;
+
+ if (!ProcessIterationStmt(S, Third, LoopIncrement, LoopDRE)) return;
+ if (!ProcessIterationStmt(S, LastStmt, LastIncrement, LastDRE)) return;
+
+ // Check that the two statements are both increments or both decrements
+ // on the same varaible.
+ if (LoopIncrement != LastIncrement ||
+ LoopDRE->getDecl() != LastDRE->getDecl()) return;
+
+ if (ContinueFinder(S, Body).ContinueFound()) return;
+
+ S.Diag(LastDRE->getLocation(), diag::warn_redundant_loop_iteration)
+ << LastDRE->getDecl() << LastIncrement;
+ S.Diag(LoopDRE->getLocation(), diag::note_loop_iteration_here)
+ << LoopIncrement;
+ }
+
} // end namespace
StmtResult
@@ -1429,6 +1527,7 @@
}
CheckForLoopConditionalStatement(*this, second.get(), third.get(), Body);
+ CheckForRedundantIteration(*this, third.get(), Body);
ExprResult SecondResult(second.release());
VarDecl *ConditionVar = 0;
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
