On Thu, Apr 16, 2015 at 12:51:33AM +0200, Arnaud Bienner wrote:
> Hi,
>
> I've submitted a patch to bug 62182 [1], and I would like to have some
> feedback about it (this is still WIP as noted in the bug).
> As it is my first patch to gcc, I'm not sure what is the best way to
> discuss/review patches (here or in bugzilla).
> Anyway, please let me know your thoughts :)
Thanks for working on this. Several comments:
- Do you have a copyright assignment on file? (Not sure if it's needed for
this particular 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.
- 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
