views:

626

answers:

7

This code illustrates something that I think should be treated as bad practice, and elicit warnings from a compiler about redefining or masking a variable:

#include <iostream>

int *a;

int* f()
{
  int *a = new int;
  return a;
}

int main()
{
  std::cout << a << std::endl << f() << std::endl;
  return 0;
}

Its output (compiled with g++):

0
0x602010

I've looked at a couple references (Stroustrup and The Complete C++ Reference) and can't find anything about when and why this is allowed. I know that it's not within a single local scope, though.

When and why is this allowed? Is there a good use for this construct? How can I get g++ to warn me about it? Do other compilers squawk about it?

+11  A: 

As to why this is allowed: this is perfectly valid.

When you are within your f() function, you're defining a local scope. Local scopes override the global scope, so defining your "a" variable there "hides" the global int *a;

Reed Copsey
+2  A: 

A lot of languages allow this sort of thing.
Usually (in relation to all languages) the most locally defined variable is the one you are referring too. Of the 20+ languages I have used this is very common.

Also most languages allow you to explicitly refer to the one in the outer scope.
For example C++ alows you to specify the variable in global scope with the :: operator.

#include  <iostream>


int a = 5;
int main()
{
    int a = 6;

    std::cout << a << "\n" << ::a << "\n";
            // Local
                           // global
}
Martin York
"All languages allow this sort of thing". The programming language on my TI-85 calculator does not. All variables are global, and there is no name hiding.
Steve Jessop
.NET (VB, C#) doesn't allow this too!
Dario
C#'s ban on shadowing exasperates me to no end when I am using higher-order functions.
Anton Tykhyy
As far as I know, BASIC languages (VB, built-in TI programs) have no real definition of scope and that is why no shadowing is allowed.
Dunya Degirmenci
+5  A: 

This is perfectly valid, but I think that with -Wall you only get a warning when you shadow a parameter.

If you want warnings when you shadow any type of variable, You can use this, from the g++ manual page:

   -Wshadow
       Warn whenever a local variable shadows another local variable, 
       parameter or global variable or whenever a built-in function is 
       shadowed.

Note that -Wshadow isn't included in -Wall by default.

tgamblin
+2  A: 

It's called shadowing, and you can specify -Wshadow to make GCC emit warnings about it:

g++ main.cpp -Wshadow

-Wall will not enable this warning.

With Microsoft's compiler, I could not get a warning, even on Level 4.

This is allowed because there is no ambiguity. Obviously two int's named 'a' in the same scope create an ambiguity (a = 10, which a?)

That said, I think using global variables (which is what's giving you the opportunity to shadow) is worse.

The only "legit" time I have seen this be a problem is when a class has private variable (like std::string name) and in a function somewhere that also uses a variable called "name". Now using name will use the local scope "name" as opposed to the probably intended private "name".

This is of course fixed by having a convention, such as either starting or ending private variable names with an underscore ("_name", "name_"), or some other convention. ("m_name").

GMan
+1  A: 

It's allowed so that you can safely ignore global identifier overriding. Essentially, you only have to be concerned with global names you actually use.

Suppose, in your example, f() had been defined first. Then some other developer added the global declaration. By adding a name, f() which used to work, still works. If overriding was an error, then the function would suddenly stop working, even though it doesn't do anything at all with the newly added global variable.

TokenMacGuy
A: 

As others have mentioned, this is perfectly legal, and is unambiguous to the compiler.

However, it's one of many features in programming languages which has the potential to cause confusion or hard-to-find bugs. Since it would be trivial to give different names to each of these variables, for the sake of clarity, I'd always suggest doing so.

Steve Melnikoff
A: 

To answer when this is allowed: basically in any two nested scopes.

For instance:

void foo() {
    int a;
    {
        int a;
    }
}

class Base {
    int a;
};
class Derived: public Base {
    int a; // Yes, the name Base::a is visible in the scope of Derived, even if private
};

class Foo() {
    int a;
    Foo(int a) : a(a) { } // Works OK
};

using std::swap;
void swap(MyClass& lhs, MyClass& rhs);
// Not strictly a variable, but name lookup in C++ happens before determining 
// what the name means.

Now, the answer must clearly be that having two 'things' with a single name in the same scope is generally allowed. This is possible because at most one of the names is actually defined in that scope; others would be merely visible in that scope. Name resolution rules determine which name is chosen, if there are multiple candidates.

You really do not want to give a warning for every case where the compiler picks between alternatives. That will give you tons of warnigns, on such innocent things as overloading and some smart template code.

MSalters