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

Richard Trieu rtrieu at google.com
Tue Apr 17 15:32:31 PDT 2012


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 remember why now.  Besides cast expressions and paren expressions,
conditional operators can get between an lvalue-to-rvalue cast and a
DeclRefExpr.

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

Couple of updates:
Got rid EvalDecl and changed how lvalue-to-rvalue casts are handled.
Moved caret location to first decl in conditional expression.
If too many things need to be underlined, underline the whole condition
expression.
Skip checking if there is a dereference in the conditional.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120417/a79ace10/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: loop-analysis4.patch
Type: application/octet-stream
Size: 14716 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120417/a79ace10/attachment.obj>


More information about the cfe-commits mailing list