Re: unused comparison warning (bug 62182)

[Marek Polacek <polacek at redhat dot com>]

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