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

[Adam Nohejl]

Hi,

On 30.10.2010, at 0:45, Axel Naumann <Axel.Naumann at cern.ch> wrote:
> On 10/21/10 7:38 PM, John McCall wrote:
>> On Oct 21, 2010, at 8:17 AM, Douglas Gregor wrote:
>>> On Oct 21, 2010, at 2:43 AM, Axel Naumann wrote:
>>>> struct A {
>>>> static void* f; void* g;
>>>> struct B { static void* f; void* g; };
>>>> };
>>>> struct C { static void* f; void* g;};
>>>> struct D: public C { static void* f; void* g; };
>>>> 
>>>> compiled with
>>>> clang++ -c -Wall -Wshadow -O2 t.cxx
>>>> generates the warning
>>>> 
>>>> t.cxx:3:28: warning: declaration shadows a static data member of 'A'
>>>> [-Wshadow]
>>>> struct B { static void* f; void* g; };
>>>>                        ^
>>>> t.cxx:2:17: note: previous declaration is here
>>>> static void* f; void* g;
>>>>             ^
>>> 
>>> It doesn't seem at all useful for Clang to warn about static data members shadowing something else by the same name. Personally, I'd limit -Wshadow to only complain when a local declaration shadows something from the outer scope.
>> 
>> I agree.
> 
> You mean like in the attached patch? The corresponding tests still to be
> updated (removed, mostly), i.e. please don't commit :-)

Incidentally, I have just added a comment to bug 6808 (other false -Wshadow warnings) yesterday.

I'm attaching a patch that combines the above changes (as Axel just removed a few things) and also prevents false warnings (1), (2), (3) in the following snippet:

class A{
    int a;
    static void f(int a){ }           // (1) false warning (in contrast to gcc)
    class X{
        X(int a){ }                   // (2) false warning (in contrast to gcc)
        static void g(int a){ }       // (3) false warning (in contrast to gcc)
    };
};

I'm new to Clang, so could you please doublecheck the code and commit it for me, if it's OK?

I also wonder whether (5), (6), and (7) in this code should produce a warning:

class X{
    int a;
    class X{
        enum{ a = 0 };                // (4) no warning (OK)
        X(int a){ }                   // (5) missing warning? (in contrast to gcc)
        static void g(int a){ }       // (6) missing warning? (like gcc)
    };
};

class Y{
    static int a;
    class X{
        enum{ a = 0 };                // (7) missing warning?
    };
};


-- 
Adam
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang_Wshadow_staticmeths_and_ctors.diff
Type: application/octet-stream
Size: 2340 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20101030/6ffa5c21/attachment.obj>