This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 18/348] Fix -Wsahdow warnings
> 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.