tags:

views:

188

answers:

6

I've been having big problems in reproducing and finding the cause of a bug. The occurence seems entirely random, so I suspected an uninitialized variable somewhere. But then I found this piece of code:

CMyClass obj; // A
obj.DoStuff();

if ( somebool )
{
    CMyClass obj; // B
    obj.DoStuff();
}

obj.DoOtherStuff();

It seems as though DoOtherStuff() is either done on "B", or that B.DoStuff() sometimes actually works on A - i.e. i DoStuff() is actually called on the first obj.

Could this happen? I don't think I got a compiler warning (I've fixed the code now in hoping that it might help). It seems very likely that this piece of the actual code is where the bug I'm trying to find is, but there could of course be other reasons I haven't discovered yet.

+1  A: 

No, it cannot happen.

A compiler guarantees that the B object is properly destroyed when exiting the if scope, and while alive it acts on its own address space.

The error is somewhere else.

Valentin Galea
+5  A: 

The code, as written, should work. The first call to DoStuff() and the last call to DoOtherStuff() can only be sent to A.

The call to DoStuff() inside the if(somebool) { } block can only be sent to B.

From the standard:

3.3.2 Local scope

  1. A name declared in a block (6.3) is local to that block. Its potential scope begins at its point of declaration (3.3.1) and ends at the end of its declarative region.

And:

3.3.7 Name hiding

  1. A name can be hidden by an explicit declaration of that same name in a nested declarative region or derived class (10.2).

That being said, perhaps this is not what was intended by the author of that code. If the variables have the same name, perhaps the intent is to have only one instance of that variable, and the B instance created inside the loop is a mistake. Have you gone through the logic to see if a second instance makes sense?

e.James
Yep, two instances was intended.
Srekel
Interesting. The decision to give them the same name is suspect. Of course, without seeing the logic, I have no idea why the author chose to do it that way. Has renaming them fixed your problems?
e.James
I doubt naming them the same thing was anything more than a copy-paste "mistake". I have not yet tested the code so not sure if it works yet.
Srekel
+3  A: 

Unless obj.DoStuff() makes a change to a some global object then as Valentin points out it should all be self contained within the scope of the if statement

ChrisF
Or some class static variables are affected. Or they share some common object which is obtained from some factory.
Mykola Golubyev
@Mykola - good points!
ChrisF
+2  A: 

Local variables with the same name as a global variable hide the global variable inside that block. However, the global scope operator (::) can be used to tell the compiler you mean the global version

CMyClass obj; // A
obj.DoStuff();
if ( somebool )
{ 
 CMyClass obj; // B  
 obj.DoStuff();  //does on B
 ::obj.DoStuff(); //does on A
}
obj.DoOtherStuff(); //this will call A because B is destroyed

So the local variable B is destroyed when it is out of scope. Having local variables with the same name as global variables is usually a call for trouble, so try avoiding whenever possible.

TStamper
+2  A: 

If you use GCC you can (and - in my opinion - should) use -Wshadow, which will emit warnings in cases such as the one in your example, which often case very subtle bugs.

However, like many other people already said, the code you pasted is correct, does what you expect, and is not undefined behavior.

Andreas Bonini
A: 

Your code may be syntactically correct, but what do the methods do? A stack overflow could cause spooky behavior such as you are observing, for instance.

dontcallmi