From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15227 invoked by alias); 25 Nov 2011 15:11:52 -0000 Received: (qmail 15216 invoked by uid 22791); 25 Nov 2011 15:11:50 -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; Fri, 25 Nov 2011 15:11:37 +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 pAPFBDkX004371; Fri, 25 Nov 2011 16:11:13 +0100 (CET) Received: (from kettenis@localhost) by glazunov.sibelius.xs4all.nl (8.14.5/8.14.3/Submit) id pAPFBBkP007292; Fri, 25 Nov 2011 16:11:11 +0100 (CET) Date: Fri, 25 Nov 2011 15:11:00 -0000 Message-Id: <201111251511.pAPFBBkP007292@glazunov.sibelius.xs4all.nl> From: Mark Kettenis To: eliz@gnu.org CC: brobecker@adacore.com, andrew.smirnov@gmail.com, gdb-patches@sourceware.org In-reply-to: <83hb1s8l5c.fsf@gnu.org> (message from Eli Zaretskii on Fri, 25 Nov 2011 14:02:55 +0200) Subject: Re: [PATCH 18/348] Fix -Wsahdow warnings References: <201111231640.pANGefc4031803@d06av02.portsmouth.uk.ibm.com> <201111231820.40486.pedro@codesourcery.com> <201111232023.pANKNcLf022983@glazunov.sibelius.xs4all.nl> <20111124220057.GU13809@adacore.com> <83hb1s8l5c.fsf@gnu.org> 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/msg00696.txt.bz2 > Date: Fri, 25 Nov 2011 14:02:55 +0200 > From: Eli Zaretskii > > > Date: Thu, 24 Nov 2011 14:00:57 -0800 > > From: Joel Brobecker > > Cc: Mark Kettenis , gdb-patches > > > > 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.