[cfe-dev] Patch: Re: -Wshadow rationale for data members

[John McCall]

On Nov 11, 2010, at 3:34 PM, Adam Nohejl wrote:
> I decided to do a more thorough revision of what -Wshadow so that its behavior makes more sense. Any comments on the following are welcome:
> 
> (Note: By "shadow" in the folliwing I mean "be reported to shadow".)
> 
> 1. What can currently shadow what
> =================================
> 
> 1.1. Warnings are issued for variable declarations (VarDecl) shadowing some other variables (local or global), fields, and static data members.

Okay.

> 1.2. There are some false warnings that can be eliminated easily (the patch I have posted earlier in this thread).
> 
> 1.3. The comment above CheckShadow() says "Diagnose variable or built-in function shadowing", which seems no longer to be the case. (I don't know what the original code was like, but now redeclaration of a builtin triggers an error, and it is not handled by CheckShadow().) The comment is outdated, right?

Yes.

> 2. What currently cannot shadow or be shadowed
> ==============================================
> 
> 2.1. A type declaration cannot shadow or be shadowed. OK?
> 
> 2.2. A function cannot shadow or be shadowed. OK?
> 
> 2.3. A namespace cannot shadow or be shadowed. OK?

These all seem reasonable.

> 2.4. Fields or static data members cannot shadow. This is worth considering: Do you agree that shadowing an accessible field of a base class with another field is worth reporting?

Yes, this is definitely a goal of -Wshadow.

> What about static data members: I think that only a static data member shadowing a member variable from a base class is worth considering. Do you agree?

Yes.

> 2.5. An enum constant cannot shadow or be shadowed. Do you agree that enum constants should be treated similarly to variables or, within a class/structure context, to static data members?

I think we shouldn't warn about shadowing enumerators.

John.