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

[Eli Friedman]

On Mon, Feb 6, 2012 at 2:50 PM, Richard Trieu <rtrieu at google.com> wrote:
> On Mon, Feb 6, 2012 at 2:02 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>>
>> On Mon, Feb 6, 2012 at 1:55 PM, Richard Trieu <rtrieu at google.com> wrote:
>> > The motivation of this path is to catch code like this:
>> >
>> > for (int i = 0; i < 10; ++i)
>> >   for (int j = 0; j < 10; ++i)
>> >     { }
>> >
>> > The second for loop increments i instead of j causing an infinite loop.
>> >  The
>> > warning also checks the body of the for loop so it will trigger on:
>> >
>> > for (int i; i <10; ) { }
>> >
>> > But not trigger on:
>> >
>> > for (int i; i< 10; ) { ++i; }
>> >
>> > I'm still fine-tuning the trigger conditions, but would like some
>> > feedback
>> > on this patch.
>>
>> Adding an additional recursive traversal of the body of every parsed
>> for loop is very expensive.
>>
>> -Eli
>
>
> Is it that expensive?  I would think that once the AST was constructed, the
> visitor would be pretty fast, especially if no action is taken on most of
> the nodes.  I also made the warning default ignore and put in an early
> return to prevent the visitors from running in the default case.
>
> Do you have any suggestions on removing the recursive traversal?

Okay, thinking about it a bit more, maybe it's not that expensive, but
you should at least measure to make sure.

I don't have any good suggestion for an alternative.

-Eli