Re: [PATCH 18/348] Fix -Wsahdow warnings

[Andrey Smirnov <andrew.smirnov@gmail.com>]

On Thu, Nov 24, 2011 at 3:23 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> From: Pedro Alves <pedro@codesourcery.com>
>> Date: Wed, 23 Nov 2011 18:20:40 +0000
>>
>> On Wednesday 23 November 2011 16:40:41, Ulrich Weigand wrote:
>> > Mark Kettenis wrote:
>> >
>> > > > From: Andrey Smirnov <andrew.smirnov@gmail.com>
>> > > > Date: Tue, 22 Nov 2011 17:25:56 +0700
>> > > > Subject: [PATCH 18/39] Fix -Wshadow warnings.
>> > > >
>> > > > * amd64-linux-tdep.c (amd64_canonicalize_syscall): Fix -Wshadow
>> > > >         warnings.
>> > >
>> > > Why the hell does -Wshadow complain here?
>> >
>> > > > -amd64_canonicalize_syscall (enum amd64_syscall syscall)
>> > > > +amd64_canonicalize_syscall (enum amd64_syscall syscall_number)
>> >
>> > I'd expect this is because the parameter "syscall" shadows the global
>> > function declaration "syscall" provided by glibc headers:
>> >
>> > /usr/include/unistd.h:extern long int syscall (long int __sysno, ...) __THROW;
>>
>> Yeah, this is unfortunate because it means you trigger different
>> shadows on different hosts, or by configuring gdb differently.
>
> Indeed.  And I'd say this means we can't add -Wshadow to the set of
> default flags for compiling gdb.

IMHO,
not setting the -Wshadow variable to the default set would only be
inviting for such warnings to accumulate and grow into snowball of
little instances where aforementioned rule has been violated, that
would require yet another 300+ patches to clean it up. Doing so would
be plugging the wholes instead of fixing the problem.

I'll admit you annoyed me real good when you started kicking all my
toys and called them stupid, and I apologize if my behavior is
childish, but now I see your argument as this:
"We can't have all the pretty names on for our variables on all the
platforms because of the -Wshadow, let's not use -Wshadow", maybe I'll
cool down and see the merit of your argument. Now I don't.

>> There was this gcc patch
>>
>>  http://comments.gmane.org/gmane.comp.gcc.patches/244771
>>
>> to stop -Wshadow from complaning about shadowing of symbols in system
>> headers, but it doesn't seem to have been applied, though it was okayed.
>
> I'd argue that -Wshadow should never warn about a local variable
> shadowing a function.  There'sno chance such shadowing is
> unintentional.

"There'sno chance such shadowing is unintentional." is either an
assumed conclusion or an argument from incredulity both of which are
logical fallacies, but even if the shadowing is always intentional I
still fail to see your point. You are trying to use the name that has
already been taken, since both the names of the functions and
variables reside in the same namespace it is an name clash. There is,
to the best of my knowledge, no means in the C language to help
compiler to resolve this ambiguity and when you ask compiler to slap
you for any naming ambiguities with -Wshadow you get what you asked
for. Why shouldn't it be so?

> It seems a lot of the changes posted up until now are dealing with
> that type of "conflict".  I'd really like to see those dropped from
> this set.
>

It is not a "conflict". It is a conflict. Dropping such changes would
make it impossible to test wither the whole goal of the patch-set,
that is building with -Wshadow enabled, has been achieved.

Andrey Smirnov