From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5900 invoked by alias); 25 Nov 2011 16:26:44 -0000 Received: (qmail 5884 invoked by uid 22791); 25 Nov 2011 16:26:41 -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 16:26:27 +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 pAPGQ5Ai020378; Fri, 25 Nov 2011 17:26:05 +0100 (CET) Received: (from kettenis@localhost) by glazunov.sibelius.xs4all.nl (8.14.5/8.14.3/Submit) id pAPGQ3rN016558; Fri, 25 Nov 2011 17:26:04 +0100 (CET) Date: Fri, 25 Nov 2011 16:26:00 -0000 Message-Id: <201111251626.pAPGQ3rN016558@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: <83ehww8b1t.fsf@gnu.org> (message from Eli Zaretskii on Fri, 25 Nov 2011 17:41:02 +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> <201111251511.pAPFBBkP007292@glazunov.sibelius.xs4all.nl> <83ehww8b1t.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/msg00702.txt.bz2 > Date: Fri, 25 Nov 2011 17:41:02 +0200 > From: Eli Zaretskii > > > Date: Fri, 25 Nov 2011 16:11:11 +0100 (CET) > > From: Mark Kettenis > > > > > 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. > > But the patches suggested by Andrey didn't find any instances of the > above. So I submit that this danger is purely theoretical at this > point, or at least sufficiently rare in GDB to render this > consideration not important in practice for us. You're missing my point. A significant number of Andrey's changes (all-but-one of the ones I looked at) rename *local* variables because they have the same name as a function. My example above tries to show you that there is absolutely no problem with that, because if one wouldf accidentally try to invoke the function that has the same name as the local variable, the compiler would already generate an error. > > > 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. > > Clashing with library symbols _is_ a good reason to prevent GDB from > building. For *local* variables? > > And remove some perfectly usable and meaningful variable names. > > They can be easily replaced by no less meaningful and usable ones. But such a change requires one to actually read and understand the code. Such changes can therefore never be obvious, so they can't be fixed quickly. > > > 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! > > The same is true of any code you add or change, with the possible > exception of very trivial changes. For most changes, compiling on one system gives you reasonable confidence that things work on other systems as well. > > Most people that contribute to GDB are fairly competent programmers. > > Being competent doesn't mean being clairvoyant. How can we trust > ourselves to know by heart every single symbol in the standard > libraries? Exactly! That's why -Wshadow is so bad. Our "defs.h" pulls in lots of system headers. And on top of that we use options like _GNU_SOURCE, which invite the system to pollute the global namespace as much as it want. Since there is absolutely no problem with a local variable that has the same name as a library function, this is bad.