6
votes

I accidentally shadowed some member variables of a (base) struct with private members in a class derived by the base struct.

struct Base {
int a;
}

class Derived : public Base {
private:
int a;
...

That was a mistake in my case, causing a sneaky bug (luckily caught while testing).
As I think that shadowing members on purpose is really rare (if not considered bad practice at all), I wonder why the compiler is not raising at least a warning (ok, not an error because shadowing is legally permitted)?

The compiler I used is Microsoft Visual C++ 2015, warning level 4).
I wonder if other compilers (i.e. GCC) provide a specific warning for this situation?

1
gcc is also not providing warning, but interesting, how this behavior should work overall with polymorphismStarl1ght
I believe that shadowing a variable like in your example, even without private, is almost always a mistake. I filed a feature request both for clang and gcc.Vittorio Romeo
@VittorioRomeo It is not a mistake if a was not present in Base at the time Derived was written.Raymond Chen
@Raymond Chen, just for your information, a was already present in Base in my case.roalz
@roalz: The compiler won't know the historic circumstances that led to the situation it sees when compiling the code. There are valid cases, where this is completely safe. A compiler cannot warn without knowing the history of both classes.IInspectable

1 Answers

7
votes

Whether shadowing is bad or good depends on the order in which you introduced the conflicting names.

Suppose you have a class library, and one of the classes is this:

struct Base {
    int a;
};

Later, customer A who is using your class library writes this:

class DerivedA : public Base {
private:
    int a;
};

In this case, shadowing is probably unintended. The customer has accidentally shadowed Base::a.

However, suppose you also have customer B, who writes this:

class DerivedB : public Base {
private:
    int b;
};

So far so good. Now you build up your library so it uses Base objects, and customer B who uses your library builds up a body of code that uses both Base and DerivedB objects.

A few weeks later, you realize that in order to get a new feature, you need to add a new member to Base.

struct Base {
    int a;
    int b; // new member variable
};

Does this create a problem with your library? Does it create a problem with customer B?

No, it doesn't create any problems.

All of your code that uses Base will continue to use Base, and it can use the b member to get the fancy new b feature. Even if the DerivedB object is passed to a function that expects a Base, the fact that Derived is shadowing b has no effect on Base. Your function that uses Base can say b and it will access the Base member variable.

Meanwhile, all of customer B's code that uses DerivedB will continue to use DerivedB, and when that code says b, it gets DerivedB::b, just like it did before. Phew, shadowing saved the day!

(Of course, if customer B wants to start taking advantage of the new b feature, then customer B has to do extra work to resolve the conflict. But the important thing is that the shadowing didn't create any new problems in existing code.)

At the end of the day, whether shadowing is good or bad depends on the order in which you introduced the conflicting names. This is not something a compiler has insight into.