This is the mail archive of the
mailing list for the GDB project.
Re: Notes on -Wshadow patches
- From: Mark Kettenis <mark dot kettenis at xs4all dot nl>
- To: eliz at gnu dot org
- Cc: andrew dot smirnov at gmail dot com, gdb-patches at sourceware dot org, brobecker at adacore dot com, tromey at redhat dot com
- Date: Sun, 27 Nov 2011 15:14:44 +0100 (CET)
- Subject: Re: Notes on -Wshadow patches
- References: <email@example.com> <E1RUeSM-0003Dc-Jd@fencepost.gnu.org>
> Date: Sun, 27 Nov 2011 08:07:42 -0500
> From: Eli Zaretskii <firstname.lastname@example.org>
> > - At least 1 in 3 detected conflicts was a conflict of either type ALBW or
> > BWHD, both of which, I hope we can all agree, are either
> > unintentional mistake or a hack too clever for it's own good, which
> > makes the later editing of the code into "Operation" game.
> I actually think that both ALBW and BWHD are perfectly valid C code,
> and neither unintentional mistakes nor "too clever hacks". The only
> type of clashes that should be fixed are TYP2.
It's all perfectly valid C code. That's not the point. The point is
that certain types of shadowing are indicative of bugs in the code.
In case of the ALBW type, the potential bug is that the programmer
intends to modify the variable in the outer scope, which doesn't
happen because a variable with the same name exists in a nested scope.
I can remember a few bugs like that in the past decade of GDB
development. So those are worth fixing. It's somewhat less likely to
be a bug in the BWDH case, but I think those should be fixed as well
(by renaming the global variable, turning it into a static variable,
or perhaps by getting rid of the global variable completely). And
fixing the TYP1 and TYP3 cases is probably a good idea as well, since
it will help to make the code more readable. A -Wshadow option that
only warns about these 4 types would actually be useful.
The TYP2 case will never run the risk of being a bug since the
distinction between a function invocation and variable usage in the C
language is unambigious.