Re: [boost] Warning policy? local variable hides (i.e. shadows) global variable

Subject: Re: [boost] Warning policy? local variable hides (i.e. shadows) global variable
From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2015-01-15 08:49:59

On Thu, Jan 15, 2015 at 4:11 PM, Beman Dawes <bdawes_at_[hidden]> wrote:
> GCC and Clang have had a warning for years (-Wshadow): "Warn whenever
> a local variable or type declaration shadows another variable,
> parameter, type, class member (in C++), or instance variable (in
> Objective-C) or whenever a built-in function is shadowed. Note that in
> C++, the compiler warns if a local variable shadows an explicit
> typedef, but not if it shadows a struct/class/enum."
>
> VC++ 2015 (aka 14.0) Preview has now added a similar warning, C4459.
> For example, "c:\boost\modular\develop\boost\lexical_cast\detail\converter_lexical_streams.hpp(429):
> warning C4459: declaration of 'n' hides global declaration". The VC++
> warning also provides location info so is easy to view both the
> declarations involved. In this case, the declaration "bool
> operator<<(short n)" is shadowing a variable name in the unnamed
> namespace of the calling translation unit.
>
> The process of clearing these shadow warnings occasionally finds bugs
> that are otherwise difficult to detect. Peter Dimov and I have both
> found bugs in our code in the process of clearing the new C4459
> warning. Even though most of the warnings don't actually signify bugs,
> I'm finding that clearing them makes code clearer and less confusing,
> particularly code I haven't looked at in a long while.
>
> Should Boost have policy to clear these warnings?
>
> A lot of the warnings involve function argument names. Should we have
> a guideline to prevent shadow warnings?  A convention for argument
> names would make it easier to submit pull requests. Possible
> guidelines:
>
> * Prefix function argument names with "a_". Rationale: The "m_" prefix
> for member names has been a success.
> * Suffix function argument names with "_". Rationale: Short and less
> distracting than "m_" prefix.
>
> Thoughts?

Despite that the warnings should probably be fixed, I don't think
there is need for a naming policy. I'm sure any guideline will
contradict someone's habits or preferences, and will be difficult to
maintain. Besides it's not clear what to do with tons of written code
- starting a new convention without converting the existing code is
hardly an improvement.

As for the particular issue, it's probably better to mangle the global
variable names (my preference is "g_") rather than function arguments
or local variables. Globals are much more rare and their names should
be chosen carefully anyway.