Re: [PATCH 2/3] Change location_to_sals to a method

[Pedro Alves <pedro@palves.net>]

On 2022-06-01 05:50, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
> 
>>> int thread, task;
> 
> Pedro> These thread/task are masking the breakpoint fields, and then ...
> 
> Ugh, I even looked for this.
> 
> I looked at turning on -Wshadow instead of -Wshadow=local, since that
> would fix this problem.  It's a bit of a slog, and we'd be at the mercy
> of system headers too, but maybe we want to do this?

ISTR that we tried -Wshadow early on, and it ran into lots of issues with
system headers.  Like, e.g., there's a system function named "index" in strings.h,
which is easy to run into.  

And then there were also with issues with variables shadowing functions.

ISTR Alan filing a GCC bug about it, to ask to not warn about shadowing
system header stuff, or maybe it was about not warning about variables
shadowing functions.  

/me searches.

Ah, here, it was actually a GCC fix:

  https://gcc.gnu.org/legacy-ml/gcc-patches/2011-08/msg02017.html

That predates gcc 4.8, I think, our oldest supported GCC version,
so we can assume all GCCs have that.  

OTOH, I'm not sure that patch affects only the C frontend, or whether
that code is shared with the C++ frontend.  g++ maybe behaves differently,
but hopefully not.

AFAICS, bfd uses -Wshadow, not -Wshadow=local.

Searching in my gdb-patches folder, I found a ton of -Wshadow patches,
from Andrey Smirnov from back in 2011.  I had forgotten how many those were.
It was a series with over 300 patches.  I think in the end most were dropped,
for handling system header conflicts, which was found not worth it.

Maybe we could give -Wshadow another try.

> 
> Meanwhile I filed a gcc feature request to add a new -Wshadow that is
> like local but also handles data members:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105798
> 
> Tom
>