[boost] Re: Regex Shadow Warnings

From: Stephan T. Lavavej (stl_at_[hidden])
Date: 2003-11-16 19:25:15

[David Abrahams]
> This is an example of one which often flags perfectly good code.

[Stephan T. Lavavej]
> Code with shadowed variables can be completely correct, but shadowing is
> still risky.  Thus, I /don't/ think that code which -Wshadow complains
> about is perfectly good code.
> Shadowing adds zero power or convenience, but does add danger.

[David Abrahams]
> It depends if you're shadowing some inherited implementation detail
> which you didn't put there and don't care about.

You may not always be responsible for the original variable, but you are
responsible for the variable doing the shadowing. Is it really so hard to
give it a different name?

In the specific case of Boost Regex, all of the code involved in the
shadowing is Boost's.

[David Abrahams]
> Should we try to come to a concensus about this?

[Stephan T. Lavavej]
> A decision one way or the other would be nice.
>
> If you decide that Boost shouldn't be -Wshadow clean, then at the very
> least it would be nice to add a #pragma GCC system_header to each header
> file when they're being used by users and not developers.  Otherwise,
> using Boost will make -Wshadow useless, which is just as bad as, say,
> polluting the user's namespace.

[David Abrahams]
> Be happy to, once you convince the GCC developers to start supporting
> #pragmas ;-)

Huh?  #pragma GCC system_header is supported by GCC; that's why I mentioned
it.  See: http://gcc.gnu.org/onlinedocs/gcc-3.3.2/cpp/Pragmas.html#Pragmas

[Stephan T. Lavavej]
> The warnings that I compile my own code with are:
>
> -Wall -W -Wfloat-equal -Wshadow -Wpointer-arith -Wcast-qual -Wcast-align
> -Wwrite-strings -Wconversion -fno-nonansi-builtins -Wold-style-cast
> -Woverloaded-virtual

[David Abrahams]
> OK, but what makes your set of warnings the right set?

I mentioned my set of warnings only as an example of one possible set.
Boost should come to a consensus about what warnings its libraries should be
compiled with.

Even if Boost's developers are comfortable with code that uses shadowing,
using shadowing in Boost forces this decision on all of Boost's users, and
that isn't nice.  It's the Wrath of Khan principle: The needs of the many
outweigh the needs of the few... or the one.

Stephan T. Lavavej
http://nuwen.net