Why does the compiler warn about shadowing a member in initalization lists?

Question

Take a constructor parameter with the same identifier as the data member it's initializing. If the two are used inside an initialization list, it would be considered safe, and "shadowing" would not occur.

However, take the following example:

struct A{
  A(int a, int b);
  int a, b;
};

A::A(int a, int b)
: a(a)
, b(b)
{}

int main(){}

warnings seen with g++:

g++ -Wshadow --std=c++1z -o main main.cpp 
main.cpp: In constructor ‘A::A(int, int)’:
main.cpp:8:18: warning: declaration of ‘b’ shadows a member of ‘A’ [-Wshadow]
 A::A(int a, int b)
                  ^
main.cpp:4:10: note: shadowed declaration is here
   int a, b;
          ^
main.cpp:8:18: warning: declaration of ‘a’ shadows a member of ‘A’ [-Wshadow]
 A::A(int a, int b)
                  ^
main.cpp:4:7: note: shadowed declaration is here
   int a, b;

warnings seen with clang:

clang++ -Wshadow --std=c++1z -o main t.cpp
main.cpp:8:10: warning: declaration shadows a field of 'A' [-Wshadow]
A::A(int a, int b)
         ^
main.cpp:4:7: note: previous declaration is here
  int a, b;
      ^
main.cpp:8:17: warning: declaration shadows a field of 'A' [-Wshadow]
A::A(int a, int b)
                ^
main.cpp:4:10: note: previous declaration is here
  int a, b;

I'm not doing anything to the data members in the body of the constructor-- so why am I getting these warnings?

I'd like to leave the warning flag enabled to catch legitimate mishaps, but these particular cases are causing too much false noise. Is there validity in these warnings?

Hello, I think there is a discussion about this here: http://stackoverflow.com/questions/268587/can-i-use-identical-names-for-fields-and-constructor-parameters — Nikita Vorontsov, Jan 20 '16 at 09:15
That's exactly what the compiler option is for, you're shadowing the declarations so it tells you as much, it's no different than `int main() { char c; {char c;} }`. — user657267, Jan 20 '16 at 09:19
@NikitaVorontsov Yes, this is very relevant. My question differs on why there is a warning, and not whether it's legal C++. — Trevor Hickey, Jan 20 '16 at 09:23

score 22 · Accepted Answer · answered Jan 20 '16 at 09:13

22

Although you aren't doing anything where the shadowing might hurt you now, you are setting yourself up for some maintenance headaches.

If Maintainer Joe comes along and and needs to make a change like this:

A::A(int a, int b)
: a(a)
, b(b)
{
    storeReference(a);
}

Oops! He's just stored a reference to the parameter rather than the data member.

The compiler isn't telling you you're doing anything wrong per se, it's telling you that you might want to change your names so that you don't have issues in the future.

I, for one, would recommend choosing a naming convention to disambiguate and sticking to it. Personally, I like to put m_ on the front of member data, but others prefer to suffix _ to the member data or the constructor param.

answered Jan 20 '16 at 09:13

TartanLlama

63,752
13
157
193

3

That's fair. I would have hoped the compiler could stay silent until Joe added something to the body that was indeed dangerous. A blank body seems trivial to detect, and common enough that a warning shouldn't emit. Perhaps that behaviour is less agreed upon than I thought. – Trevor Hickey Jan 20 '16 at 09:19
An alternative convetion (nicer according to me) is to always prefix data members with `this->`. However, your approach is more foolproof (i.e. a mistake is a compile error, whereas in my case a mistake is just a linter warning). – Martin Pecka May 09 '19 at 10:36
The compiler could easily ignore using the shadowed variable in initializer list and only fire warning if the variable is used elsewhere, but nobody in gcc dev team cares. Lately -Wshadow was imposed on our codebase, and I now have to refactor hundreds of member variables to end with underscore to fix the warning. – Radek Sep 07 '21 at 14:46

score -3 · Answer 2 · answered Jan 20 '16 at 09:08

-3

It complains because of a(a) and b(b) - the name of the struct's member shall differ from the name of the parameter to the constructor. Then the construction will look like mA(a) and mB(b)

answered Jan 20 '16 at 09:08

dmi

1,424
1
9
9

2

The first `a` is looked up in the scope of the constructor's class, it doesn't *need* a different name. – TartanLlama Jan 20 '16 at 09:17
1

I didn't vote but your comment makes it sound like the code is wrong or something (it isn't) – M.M Jan 20 '16 at 09:17
Symantically the code is correct. It is only risk prone due to the name of the destination variable is equal to the name of the source variable. And the compiler warns to pay attention whether this construction does exactly what the developer wants. – dmi Jan 20 '16 at 09:21
@dmi You might want to clarify in your answer; your current wording seems to imply that the code is invalid. – TartanLlama Jan 20 '16 at 09:22
The wording means the code is not correct, however it will compile and run. – dmi Jan 20 '16 at 09:26
@dmi What do you mean `the code is not correct`? The name lookup rules are well-defined here and the member variables `a` and `b` will be initialized with the values of the parameters `a` and `b`. – TartanLlama Jan 20 '16 at 09:29
I am talking about weak reliability, maintanability and coding style of the code. With respect to those aspects the compiler warning is absolutely legal. It can be interpreted as "Are you sure that you want to do this?". Imagine a case if someone (or even you) later remove or rename the member variables. The code will still compile and work, but its behavior will be most probably not as intendet. – dmi Jan 20 '16 at 09:38
The code will fail to compile if the member variable names are changed, because only members can appear in the *ctor-initializer* list – M.M Jan 20 '16 at 09:42

Why does the compiler warn about shadowing a member in initalization lists?

2 Answers2