Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Mark Kettenis <mark.kettenis@xs4all.nl>
To: eliz@gnu.org
Cc: andrew.smirnov@gmail.com, gdb-patches@sourceware.org,
	       brobecker@adacore.com, tromey@redhat.com
Subject: Re: Notes on -Wshadow patches
Date: Sun, 27 Nov 2011 14:15:00 -0000	[thread overview]
Message-ID: <201111271414.pAREEiBn028234@glazunov.sibelius.xs4all.nl> (raw)
In-Reply-To: <E1RUeSM-0003Dc-Jd@fencepost.gnu.org> (message from Eli Zaretskii	on Sun, 27 Nov 2011 08:07:42 -0500)

> Date: Sun, 27 Nov 2011 08:07:42 -0500
> From: Eli Zaretskii <eliz@gnu.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.


  reply	other threads:[~2011-11-27 14:15 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-27 12:27 Andrey Smirnov
2011-11-27 13:07 ` Eli Zaretskii
2011-11-27 14:15   ` Mark Kettenis [this message]
2011-11-27 20:55     ` Andrey Smirnov
2011-11-27 21:24   ` Andrey Smirnov
2011-11-27 21:52     ` Joel Brobecker
2011-11-27 17:59 ` Joel Brobecker
2011-11-27 21:17   ` Andrey Smirnov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201111271414.pAREEiBn028234@glazunov.sibelius.xs4all.nl \
    --to=mark.kettenis@xs4all.nl \
    --cc=andrew.smirnov@gmail.com \
    --cc=brobecker@adacore.com \
    --cc=eliz@gnu.org \
    --cc=gdb-patches@sourceware.org \
    --cc=tromey@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox