[cfe-dev] Proposal: -Wshadow-field flag

[David Blaikie]

On Tue, Apr 21, 2015 at 3:29 PM, Reid Kleckner <rnk at google.com> wrote:
> On Tue, Apr 21, 2015 at 3:08 PM, Ehsan Akhgari <ehsan.akhgari at gmail.com>
> wrote:
>>
>> On Tue, Apr 21, 2015 at 5:41 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>>
>>> FWIW I'd be a bit concerned that this warning, while it might have a good
>>> false positive rate on a codebase with the particular idioms you've
>>> described, would have a false positive rate a bit too high for a normal
>>> diagnostic bar.
>>>
>>> Only one way to find out, though.
>>
>>
>> That is a good point.
>>
>> FWIW, I don't think we should turn this warning on as part of -Wall, maybe
>> as part of -Wextra?  I'm not at all familiar with what the usual process for
>> turning on warnings by default looks like, so I would appreciate if someone
>> can point me in the right direction there.
>
>
> Confusingly, "on by default" and "enabled by -Wall" are distinct in Clang,
> but I don't find the distinction is not very useful.
>
> I would suggest adding the warning under -Wshadow-field, and make it a
> subgroup of -Wshadow. This way, -Wshadow users will get new warnings that
> they probably wanted anyway.

The part of this warning that I'm not sure the 'normal' user of
-Wshadow need is where it warns regardless of access control - for
most users I wouldn't imagine it's important that my base class has a
private field called 'foo' when my derived class has a field called
'foo'.

> -Wshadow is not part of -Wall, so -Wall won't
> enable it. Mark each new diagnostic as DefaultIgnore in
> DiagnosticSemaKinds.td, so that it will not be on by default.
>
> Once it's in, we can try it out, and if it has a really low false-positive
> rate, then we can discuss enabling it by default and/or putting it in -Wall.

Historically there's been a fair bit of push back on off-by-default
warnings as they tend to add weight to the compiler without much in
the way of usage or quality checks (because of a lack of usage), but
perhaps this attitude has changed.

- David