Re: A strange gcc behavior, and an argument against -Wno-unused

[Pedro Alves <pedro at codesourcery dot com>]

On Saturday 10 October 2009 01:43:35, Michael Snyder wrote:
> We have "-Wno-unused" in our Makefile.in, as a result of which
> we have accumulated a huge pool of unused variables, which
> probably get optimized away and so don't hurt anything, but
> which clutter up the code.
> 
> I was playing around with the idea of cleaning them up, 

Yeah.  I think Aleksandar's "-Wall patches" patch
series must have fixed most of these already.  Unfortunately,
the patch was so large, that it ended up dropped on
the floor.  Please, if you have patches already that
properly fix some warnings, let's put those in, even
if we don't enable more warnings by default immediately.

> and 
> so I removed the "-Wno-unused" from the makefile.
> 

(...)

> So, for discussion, should we remove -Wno-unused?

I think that would be nice.  I'll all for having
tighter warnings and leaner code.  But we should try
to be smooth, and first get rid of all the corresponding
warnings in at least an --enable-targets=all build.
It can't hurt.  That discussion you're proposing should
resolve faster if removing the switch doesn't result
in a gazillion warnings and breaking everyone's builds.

On Saturday 10 October 2009 03:39:03, Dave Korn wrote:

>  Or append "-Wunused-value"?

Sounds like a good first step to me.

Curious as I am, I tried a build with that on.  An --enable-targets=all
on x86_64-linux built with

 make WARN_CFLAGS="-Wall -Wdeclaration-after-statement -Wpointer-arith -Wformat-nonliteral -Wno-pointer-sign -Wno-unused -Wno-switch -Wno-char-subscripts -Werror -Wunused-value" -k

has only revealed this extra warning (in addition to the one
you pointed out):

 ../../src/gdb/mi/mi-cmd-stack.c: In function 'list_args_or_locals':
 ../../src/gdb/mi/mi-cmd-stack.c:265: warning: left-hand operand of comma expression has no effect

"Fixed" as below.

-- 
Pedro Alves

2009-10-10  Pedro Alves  <pedro@codesourcery.com>

	* mi/mi-cmd-stack.c (list_args_or_locals): Use internal_error.
	Put "break" statements on their own line.

---
 gdb/mi/mi-cmd-stack.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Index: src/gdb/mi/mi-cmd-stack.c
===================================================================
--- src.orig/gdb/mi/mi-cmd-stack.c	2009-10-10 16:30:30.000000000 +0100
+++ src/gdb/mi/mi-cmd-stack.c	2009-10-10 17:00:14.000000000 +0100
@@ -256,13 +256,17 @@ list_args_or_locals (enum what_to_list w
   switch (what)
     {
     case locals:
-      name_of_result = "locals"; break;
+      name_of_result = "locals";
+      break;
     case arguments:
-      name_of_result = "args"; break;
+      name_of_result = "args";
+      break;
     case all:
-      name_of_result = "variables"; break;
+      name_of_result = "variables";
+      break;
     default:
-      gdb_assert (("unexpected value", 0));
+      internal_error (__FILE__, __LINE__,
+		      "unexpected what_to_list: %d", (int) what);
     }
 
   cleanup_list = make_cleanup_ui_out_list_begin_end (uiout, name_of_result);