Re: [PATCH 18/348] Fix -Wsahdow warnings

[Mark Kettenis <mark.kettenis@xs4all.nl>]

> Date: Fri, 25 Nov 2011 14:02:55 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > Date: Thu, 24 Nov 2011 14:00:57 -0800
> > From: Joel Brobecker <brobecker@adacore.com>
> > Cc: Mark Kettenis <mark.kettenis@xs4all.nl>,	gdb-patches <gdb-patches@sourceware.org>
> > 
> > I mean, I understand that "index" might be part of a system's
> > include. But "block_found" (or was it "found_block") seems quite
> > surprising.
> 
> If it surprised you, it means -Wsahdow did its job well.

No it didn't.  It does a poor job, by flagging cases that can never
ever cause problems.  A local variable "shadowing" a global function
name will never be a problem, because something like:

void foo(void);

void
bar(void)
{
	int foo;

	foo();
}

simply won't compile, even if you don't use -Werror.

> IOW, it's easy to avoid well-known names like `printf' or `strchr'.
> It's the not-so-well-known names that give you hell and high water.

Exactly.  And since we enable -Werror by default, -Wshadow will break
GDB builds for no good reason.  And remove some perfectly usable and
meaningful variable names.

> > Add the fact that includes and compiler vary from system to system,
> > and we're not sure that once clean on one machine, it'll be clean
> > everywhere else.
> 
> But this is true of any other non-trivial piece of our code.  E.g., if
> you declare a variable `long' expecting it to be a 64-bit type, this
> will break on MS-Windows, and you will never no until you actually try
> such a compilation.

Well, that would be a wrong assumption on any sane 32-bit platform as
well, so that issue could be caught on most platforms that GDB runs on.

> In general, we have no bullet-proof way of making sure our code works
> on all supported platforms, except by actually compiling it on those
> platforms.  That's why platforms which lose their area maintainers
> bit-rot quite quickly.  This compiler switch doesn't change this
> situation in any way.

I disagree.  -Wshadow is a game changer because actually compiling on
*all* plaforms is the only way to make sure that I don't actually
break anything.  That's extremely bad!

> > All of this to fix warnings that, as far as I could tell for the
> > most part, did not indicate an actual bug in the code.

Absolutely!

> It's a bug waiting to happen, though.  That it didn't happen yet is
> just sheer luck.

Not really.  Most people that contribute to GDB are fairly competent
programmers.