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

[Richard Trieu]

On Thu, Apr 5, 2012 at 10:25 AM, Douglas Gregor <dgregor at apple.com> wrote:

>
> 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?
>

I'll give it a shot and see what happens.  There's also a few problems I
encountered in testing that I'd like to fix before committing.

>
> Otherwise, this patch seems fine. I'm curious to see how it works out in
> practice.
>
>        - Doug
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120416/57c82790/attachment.html>