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

[Eli Zaretskii <eliz at gnu dot org>]

> Date: Fri, 25 Nov 2011 16:11:11 +0100 (CET)
> From: Mark Kettenis <mark.kettenis@xs4all.nl>
> CC: brobecker@adacore.com, andrew.smirnov@gmail.com,
>         gdb-patches@sourceware.org
> 
> > 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.

But the patches suggested by Andrey didn't find any instances of the
above.  So I submit that this danger is purely theoretical at this
point, or at least sufficiently rare in GDB to render this
consideration not important in practice for us.

> > 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.

Clashing with library symbols _is_ a good reason to prevent GDB from
building.

> And remove some perfectly usable and meaningful variable names.

They can be easily replaced by no less meaningful and usable ones.

> > > 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.

What about the assumption that a long is as wide as a pointer?  Don't
dismiss a principle just because one of its examples can be easily
refuted.

> > 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!

The same is true of any code you add or change, with the possible
exception of very trivial changes.

> Most people that contribute to GDB are fairly competent programmers.

Being competent doesn't mean being clairvoyant.  How can we trust
ourselves to know by heart every single symbol in the standard
libraries?