On Tue, Mar 1, 2022 at 10:14 AM Kees Cook keescook@chromium.org wrote:
The first big glitch with -Wshadow was with shadowed global variables. GCC 4.8 fixed that, but it still yells about shadowed functions. What _almost_ works is -Wshadow=local.
Heh. Yeah, I just have long memories of "-Wshadow was a disaster". You looked into the details.
Another way to try to catch misused shadow variables is -Wunused-but-set-varible, but it, too, has tons of false positives.
That on the face of it should be an easy warning to get technically right for a compiler.
So I assume the "false positives" are simply because we end up having various variables that really don't end up being used - and "intentionally" so).
Or rather, they might only be used under some config option - perhaps the use is even syntactically there and parsed, but the compiler notices that it's turned off under some
if (IS_ENABLED(..))
option? Because yeah, we have a lot of those.
I think that's a common theme with a lot of compiler warnings: on the face of it they sound "obviously sane" and nobody should ever write code like that.
A conditional that is always true? Sounds idiotic, and sounds like a reasonable thing for a compiler to warn about, since why would you have a conditional in the first place for that?
But then you realize that maybe the conditional is a build config option, and "always true" suddenly makes sense. Or it's a test for something that is always true on _that_architecture_ but not in some general sense (ie testing "sizeof()"). Or it's a purely syntactic conditional, like "do { } while (0)".
It's why I'm often so down on a lot of the odd warnings that are hiding under W=1 and friends. They all may make sense in the trivial case ("That is insane") but then in the end they happen for sane code.
And yeah, -Wshadow has had tons of history with macro nesting, and just being badly done in the first place (eg "strlen" can be a perfectly fine local variable).
That said, maybe people could ask the gcc and clan people for a way to _mark_ the places where we expect to validly see shadowing. For example, that "local variable in a macro expression statement" thing is absolutely horrendous to fix with preprocessor tricks to try to make for unique identifiers.
But I think it would be much more syntactically reasonable to add (for example) a "shadow" attribute to such a variable exactly to tell the compiler "yeah, yeah, I know this identifier could shadow an outer one" and turn it off that way.
Linus