Thanks for your quick feedback :) 2015-04-16 10:41 GMT+02:00 Marek Polacek <pola...@redhat.com>: > - Do you have a copyright assignment on file? (Not sure if it's needed for > this particular patch.)
No I don't. Do you think I need one for this patch? > - We'll need testcases. You should e.g. ensure that the warning > works with -Wunused-comparison and doesn't show up with > -Wno-unused-comparison, > that casting to void suppresses the warning, etc. You can look into > gcc/testsuite/gcc.dg. Sure. I haven't look yet at gcc tests. I first wanted to have some feedback about this, to be sure it was worth to spend some time to implement this (i.e. this is a feature you are interested in), and that what I did wasn't completely wrong. For now, I just tested my changes manually, checking that the warning will not be Wunused-value but Wunused-comparison when the expression was an "==" equality expression. I will do the changes you mentioned, and submit a new patch. > - New options need documenting in invoke.texi. Only mentioning the new > option is not enough. See e.g. "@item -Wlogical-not-parentheses" paragraph > for an example. > - As this is a C/C++ FE warning, please move it from common.opt to > c-family/c.opt. > - Could you detail how this patch has been tested? > - Please adhere to the GNU coding style (though we can sort this out in later > reviews). > > Thanks, > > Marek
