[cfe-commits] [PATCH][PR11329][v4] Generalize -Wempty-body: warn when statement body is empty

[Argyrios Kyrtzidis]

On Jan 10, 2012, at 12:41 PM, Dmitri Gribenko wrote:

> On Tue, Jan 10, 2012 at 8:57 PM, Anna Zaks <ganna at apple.com> wrote:
>> Here is one way to implement it (suggestion by Argyrios). You can store a
>> member variable inside Sema to flag if potential warning is diagnosed. When
>> processing the next statement in ActOnExprStmt, you can check the identation
>> and make the final decision.
> 
> I think it would actually complicate the code because someone should
> be clearing that flag (and when? -- mutable state is not good...).
> And we will need to add checks not only to ActOnExprStmt, but also to
> ActOnCompoundStmt (to warn on for();{}).  This would spread the logic
> for a single warning over many functions, is it really worth doing?

Avoiding spreading the logic is a valid point, but it'd be worth it to avoid unnecessary work as much as possible.
How about at least setting a flag to indicate if an empty while/for was encountered, and if not, skip the traversing of statements altogether ?
And before setting the flag, check if the warning is disabled at that source location, which means no check is necessary at all.

Some other nitpicks are about the LLVM coding conventions:
- '{' on same line as the function declaration,
- '{'  on same line as 'if', 'for', etc.
- 'else' on same line as '}'

-Argyrios

> 
> Moving cheaper checks first: done:
> Factoring out a function: done.
> 
> Dmitri
> 
> -- 
> main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
> (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/
> <generalize-warn-empty-body-v5.patch>