r187817 - Add a new warning to -Wloop-analysis to detect suspicious increments or
Richard Trieu
rtrieu at google.com
Tue Aug 6 14:31:55 PDT 2013
Author: rtrieu
Date: Tue Aug 6 16:31:54 2013
New Revision: 187817
URL: http://llvm.org/viewvc/llvm-project?rev=187817&view=rev
Log:
Add a new warning to -Wloop-analysis to detect suspicious increments or
decrements inside for loops. Idea for this warning proposed in PR15636:
http://llvm.org/bugs/show_bug.cgi?id=15636
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/test/SemaCXX/warn-loop-analysis.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=187817&r1=187816&r2=187817&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Aug 6 16:31:54 2013
@@ -163,6 +163,7 @@ def : DiagGroup<"invalid-pch">;
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">;
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=187817&r1=187816&r2=187817&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Aug 6 16:31:54 2013
@@ -18,7 +18,12 @@ let CategoryName = "Semantic Issue" in {
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 %select{decremented|incremented}1 both 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 "
Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=187817&r1=187816&r2=187817&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Tue Aug 6 16:31:54 2013
@@ -1418,6 +1418,104 @@ namespace {
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
@@ -1444,6 +1542,7 @@ Sema::ActOnForStmt(SourceLocation ForLoc
}
CheckForLoopConditionalStatement(*this, second.get(), third.get(), Body);
+ CheckForRedundantIteration(*this, third.get(), Body);
ExprResult SecondResult(second.release());
VarDecl *ConditionVar = 0;
Modified: cfe/trunk/test/SemaCXX/warn-loop-analysis.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-loop-analysis.cpp?rev=187817&r1=187816&r2=187817&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-loop-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-loop-analysis.cpp Tue Aug 6 16:31:54 2013
@@ -152,3 +152,111 @@ void test6() {
for (;x6;);
for (;y;);
}
+
+void test7() {
+ int i;
+ for (;;i++) { // expected-note{{incremented here}}
+ if (true) test7();
+ i++; // expected-warning{{incremented both}}
+ }
+ for (;;i++) { // expected-note{{incremented here}}
+ if (true) break;
+ ++i; // expected-warning{{incremented both}}
+ }
+ for (;;++i) { // expected-note{{incremented here}}
+ while (true) return;
+ i++; // expected-warning{{incremented both}}
+ }
+ for (;;++i) { // expected-note{{incremented here}}
+ ++i; // expected-warning{{incremented both}}
+ }
+
+ for (;;i--) { // expected-note{{decremented here}}
+ if (true) test7();
+ i--; // expected-warning{{decremented both}}
+ }
+ for (;;i--) { // expected-note{{decremented here}}
+ if (true) break;
+ --i; // expected-warning{{decremented both}}
+ }
+ for (;;--i) { // expected-note{{decremented here}}
+ while (true) return;
+ i--; // expected-warning{{decremented both}}
+ }
+ for (;;--i) { // expected-note{{decremented here}}
+ --i; // expected-warning{{decremented both}}
+ }
+
+ // 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{{incremented both}}
+ }
+ for (;;i++) { // expected-note{{incremented here}}
+ if (true) break;
+ ++i; // expected-warning{{incremented both}}
+ }
+ for (;;++i) { // expected-note{{incremented here}}
+ while (true) return;
+ i++; // expected-warning{{incremented both}}
+ }
+ for (;;++i) { // expected-note{{incremented here}}
+ ++i; // expected-warning{{incremented both}}
+ }
+
+ for (;;i--) { // expected-note{{decremented here}}
+ if (true) test7();
+ i--; // expected-warning{{decremented both}}
+ }
+ for (;;i--) { // expected-note{{decremented here}}
+ if (true) break;
+ --i; // expected-warning{{decremented both}}
+ }
+ for (;;--i) { // expected-note{{decremented here}}
+ while (true) return;
+ i--; // expected-warning{{decremented both}}
+ }
+ for (;;--i) { // expected-note{{decremented here}}
+ --i; // expected-warning{{decremented both}}
+ }
+
+ // 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--;
+ }
+}
More information about the cfe-commits
mailing list