Re: unused comparison warning (bug 62182)

[Arnaud Bienner]

Thanks for your quick feedback :)

2015-04-16 10:41 GMT+02:00 Marek Polacek :
> - 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

unused comparison warning (bug 62182)

[Arnaud Bienner]

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

Arnaud

[1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62182
Index: gcc/c/c-typeck.c
===
--- gcc/c/c-typeck.c	(révision 221845)
+++ gcc/c/c-typeck.c	(copie de travail)
@@ -9921,6 +9921,10 @@ emit_side_effect_warnings (location_t lo
 {
   if (expr == error_mark_node)
 ;
+  else if (TREE_CODE (expr) == EQ_EXPR && !VOID_TYPE_P (TREE_TYPE (expr)) && !TREE_NO_WARNING (expr))
+{
+  warning_at (loc, OPT_Wunused_comparison, "equality comparison result unused: did you mean '='?");
+}
   else if (!TREE_SIDE_EFFECTS (expr))
 {
   if (!VOID_TYPE_P (TREE_TYPE (expr)) && !TREE_NO_WARNING (expr))
Index: gcc/common.opt
===
--- gcc/common.opt	(révision 221845)
+++ gcc/common.opt	(copie de travail)
@@ -727,6 +727,10 @@ Wunused-variable
 Common Var(warn_unused_variable) Warning EnabledBy(Wunused)
 Warn when a variable is unused
 
+Wunused-comparison
+Common Var(warn_unused_comparison) Warning EnabledBy(Wunused)
+Warn when the result of a comparison is unused
+
 Wcoverage-mismatch
 Common Var(warn_coverage_mismatch) Init(1) Warning
 Warn in case profiles in -fprofile-use do not match
Index: gcc/cp/cvt.c
===
--- gcc/cp/cvt.c	(révision 221845)
+++ gcc/cp/cvt.c	(copie de travail)
@@ -1345,9 +1345,15 @@ convert_to_void (tree expr, impl_conv_vo
 	  && !TREE_NO_WARNING (expr)
 	  && !processing_template_decl)
 	{
+if ((TREE_CODE (expr) == EQ_EXPR) && !TREE_NO_WARNING (expr))
+  {
+warning_at (loc, OPT_Wunused_comparison,
+"equality comparison result unused: "
+"did you mean '='?");
+  }
 	  /* The middle end does not warn about expressions that have
 	 been explicitly cast to void, so we must do so here.  */
-	  if (!TREE_SIDE_EFFECTS (expr)) {
+else if (!TREE_SIDE_EFFECTS (expr)) {
 if (complain & tf_warning)
 	  switch (implicit)
 		{
Index: gcc/doc/invoke.texi
===
--- gcc/doc/invoke.texi	(révision 221845)
+++ gcc/doc/invoke.texi	(copie de travail)
@@ -285,7 +285,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wunsuffixed-float-constants  -Wunused  -Wunused-function @gol
 -Wunused-label  -Wunused-local-typedefs -Wunused-parameter @gol
 -Wno-unused-result -Wunused-value @gol -Wunused-variable @gol
--Wunused-but-set-parameter -Wunused-but-set-variable @gol
+-Wunused-but-set-parameter -Wunused-but-set-variable -Wunused-comparison @gol
 -Wuseless-cast -Wvariadic-macros -Wvector-operation-performance @gol
 -Wvla -Wvolatile-register-var  -Wwrite-strings @gol
 -Wzero-as-null-pointer-constant}