[cfe-commits] [Patch] Add a warning for for loops with conditions that do not change

[Douglas Gregor]

On Mar 22, 2012, at 4:20 PM, Richard Trieu <rtrieu at google.com> wrote:

> New patch for the for-loop warning.
> 
> Changes:
> Both visitors are now EvaluatedExprVisitor
> The DeclExtractor, used on the conditional, works on a whitelist of Expr's instead of a blacklist.
> Fixed edge cases in the DeclMatcher with respect to LValueToRValue conversions.  Some casts aren't directly on DeclRefExprs.
> Added variable names to the warning.  Up to 4 variable names will be displayed.
> <loop-analysis3.patch>


+  void VisitCastExpr(CastExpr *E) {
+    if (E->getCastKind() == CK_LValueToRValue)
+      EvalDecl = true;
+    Visit(E->getSubExpr());
+  }

The use of EvalDecl here and here:

+  void VisitDeclRefExpr(DeclRefExpr *E) {
+    if (EvalDecl) return;
+
+    if (VarDecl *VD = dyn_cast<VarDecl>(E->getDecl()))
+      if (Decls.count(VD)) {
+        FoundDecl = true;
+        return;
+      }
+  }

feels a bit hacky. Why not look at the subexpression of an lvalue-to-rvalue cast, looking through any implicitly-generated nodes, and skip visitation if it is a DeclRefExpr?

Otherwise, this patch seems fine. I'm curious to see how it works out in practice.

	- Doug