From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4986 invoked by alias); 27 Nov 2011 14:15:30 -0000 Received: (qmail 4978 invoked by uid 22791); 27 Nov 2011 14:15:29 -0000 X-SWARE-Spam-Status: No, hits=-2.6 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from sibelius.xs4all.nl (HELO glazunov.sibelius.xs4all.nl) (83.163.83.176) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 27 Nov 2011 14:15:16 +0000 Received: from glazunov.sibelius.xs4all.nl (kettenis@localhost [127.0.0.1]) by glazunov.sibelius.xs4all.nl (8.14.5/8.14.3) with ESMTP id pAREElYo031483; Sun, 27 Nov 2011 15:14:47 +0100 (CET) Received: (from kettenis@localhost) by glazunov.sibelius.xs4all.nl (8.14.5/8.14.3/Submit) id pAREEiBn028234; Sun, 27 Nov 2011 15:14:44 +0100 (CET) Date: Sun, 27 Nov 2011 14:15:00 -0000 Message-Id: <201111271414.pAREEiBn028234@glazunov.sibelius.xs4all.nl> From: Mark Kettenis To: eliz@gnu.org CC: andrew.smirnov@gmail.com, gdb-patches@sourceware.org, brobecker@adacore.com, tromey@redhat.com In-reply-to: (message from Eli Zaretskii on Sun, 27 Nov 2011 08:07:42 -0500) Subject: Re: Notes on -Wshadow patches References: <877h2l7nth.fsf@gmail.com> 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/msg00729.txt.bz2 > Date: Sun, 27 Nov 2011 08:07:42 -0500 > From: Eli Zaretskii > > - 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.