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 11:08:15

On Thu, Jan 15, 2015 at 6:43 PM, Beman Dawes <bdawes_at_[hidden]> wrote:
>> 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.
>
> The guideline would need to be just that; a guideline that a library
> maintainer would be free to deviate from.
>
> But if there is no guideline at all, then every time a user wants to
> submit a pull request to fix the shadow warnings, it has to be
> preceded with a discussion about what to use for the particular
> library.

Right. Each library has its own conventions which have to be learned
by contributors, and that won't change whether or not you declare an
advisory guideline.

> So at least the community maintained library should try for
> consistency.

The CMT is free to agree on such a guideline for libraries it manages,
although my point wrt the existing code still stands. IMO incremental
code changes should follow naming/formatting rules imposed by the
existing code. That is unless these rules are non-existant or
absolutely detrimental for maintenance, in which case one should think
about a complete rewrite.

>> 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.
>
> The "globals" being shadowed are often in the unnamed namespace in
> user code.  So they involve simple names like "n" and "str" which
> often clash with parameter names, but the library developer has no
> control over them.

But in this case the warning is spurious since the library code uses
its parameters in either case.

We cannot realistically protect ourselves from such shadowing clashes
with the code we don't see, unless we follow STLPort route, which
prepends its names with lots of underscores. I say we shouldn't try
doing that.