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
> * 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.
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.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk