From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28538 invoked by alias); 27 Nov 2011 20:55:30 -0000 Received: (qmail 28529 invoked by uid 22791); 27 Nov 2011 20:55:30 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from mail-iy0-f169.google.com (HELO mail-iy0-f169.google.com) (209.85.210.169) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 27 Nov 2011 20:55:16 +0000 Received: by iaek3 with SMTP id k3so9920704iae.0 for ; Sun, 27 Nov 2011 12:55:15 -0800 (PST) MIME-Version: 1.0 Received: by 10.50.42.199 with SMTP id q7mr5469052igl.41.1322427315788; Sun, 27 Nov 2011 12:55:15 -0800 (PST) Received: by 10.50.186.228 with HTTP; Sun, 27 Nov 2011 12:55:15 -0800 (PST) In-Reply-To: <201111271414.pAREEiBn028234@glazunov.sibelius.xs4all.nl> References: <877h2l7nth.fsf@gmail.com> <201111271414.pAREEiBn028234@glazunov.sibelius.xs4all.nl> Date: Sun, 27 Nov 2011 20:55:00 -0000 Message-ID: Subject: Re: Notes on -Wshadow patches From: Andrey Smirnov To: Mark Kettenis Cc: eliz@gnu.org, gdb-patches@sourceware.org, brobecker@adacore.com, tromey@redhat.com Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-11/txt/msg00744.txt.bz2 On Sun, Nov 27, 2011 at 9:14 PM, Mark Kettenis wr= ote: >> Date: Sun, 27 Nov 2011 08:07:42 -0500 >> From: Eli Zaretskii > >> > =A0 - At least 1 in 3 detected conflicts was a conflict of either type= ALBW or >> > =A0 =A0 BWHD, both of which, I hope we can all agree, are either >> > =A0 =A0 unintentional mistake or a hack too clever for it's own good, = which >> > =A0 =A0 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". =A0The only >> type of clashes that should be fixed are TYP2. > > It's all perfectly valid C code. I completely agree with you, Mark, but my argument was never about standard compliance of the code. If -Wshadow was about uncovering the code that is not valid according to the standard, then both not enabling and enabling of it by default could have potentially broken the build. > 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. =A0So those are worth > fixing. =A0It'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). =A0And fixing the TYP1 and > TYP3 cases is probably a good idea as well, since it will help to > make the code more readable. =A0A -Wshadow option that only warns > about these 4 types would actually be useful. And again I completely agree with you on all of the above points(except the -Wshadow hating part :), although I would argue that both are bugs waiting to happen when someone decides to re-write/augment those pieces of code and remove inner variable, because in that case compiler won't say anything about any leftover references to it. And the case of BWDH is worse from that point of view since with ALBW you will probably be aware of shadowing. > 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. The danger I see in TYP2 clashes is not about accidental "invocation" of variable. Of course it would be reported be compiler even without -Wshadow. But what about the following example: #include #include void foo(void) { printf ("And now I am a function\n"); } struct container_element { /* ... Some stuff related to inner workings of a container ... */ void *data; }; int main(int argc, char *argv[]) { struct container_element elmnt; { char foo[42]; /* ... Lots and lots of code .... */ /* Now I am a pointer to a buffer */ elmnt.data =3D foo; } elmnt.data =3D foo; ((void (*) (void))elmnt.data)(); return 0; } How is it not of a TYP2 clash and if it is, is it not as dangerous as BWDH type? Andrey Smirnov