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

Anna Zaks ganna at apple.com
Tue Jan 10 15:20:31 PST 2012


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

This looks better after the refactor. However, maybe someone who is actively working on Sema would like to review as well/ has preference on how this should be implemented.

The only minor comment left is to correct the indentation:
+  {
should be moved to the previous line.

Cheers,
Anna.

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




More information about the cfe-commits mailing list