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

[Richard Trieu]

On Tue, Apr 17, 2012 at 3:32 PM, Richard Trieu <rtrieu at google.com> wrote:

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

Committed at r155835
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120430/01727c00/attachment.html>