views:

276

answers:

4

Hi,

I am passing a value to copy constructor as a reference, but an infinite loop is being invoked.

Here's my class:

class Vector2f{
private:
    GLfloat x;
    GLfloat y;

public:
    Vector2f();
    Vector2f(const GLfloat _x, const GLfloat _y);
    Vector2f(const Vector2f &_vector);

    ~Vector2f();
};

Here's implementation of methods:

Vector2f::Vector2f():
        x( 0.0f ),
        y( 0.0f )
{
    DebugLog("Vector2f constructor");
}

Vector2f::Vector2f(const GLfloat _x, const GLfloat _y):
        x( _x ),
        y( _y )
{
    DebugLog("Vector2f constructor(%f, %f)", _x, _y);
}


Vector2f::Vector2f(const Vector2f &_vector):
        x( _vector.getX() ),
        y( _vector.getY() )
{
    DebugLog("Vector2f copy constructor");
}

Vector2f::~Vector2f()
{

}

Here's how I access the class:

Vector2f tempVector1 = Vector2f(0.0f, 0.0f);
DebugLog("tempVector1 initialized");

Vector2f tempVector2;
tempVector2 = Vector2f(0.0f, 0.0f);
DebugLog("tempVector2 initialized");

The results I get are:

Vector2f constructor(0.000000, 0.000000)
tempVector1 initialized
Vector2f constructor
Vector2f constructor(0.000000, 0.000000)
Vector2f copy constructor
Vector2f copy constructor
Vector2f copy constructor
...

Infinite loop occurs when trying to initialize previously created object. If I try to copy tempVector1 into tempVector 2 an infinite loop occurs as well:

Vector2f tempVector2;
tempVector2 = Vector2f(tempVector1);

Why does it happen and how can I prevent it from getting into an infinite loop?

Thank you in advance.

+6  A: 

This line:

tempVector2 = Vector2f(tempVector1);

would invoke operator=, not the copy constructor. Are you defining an operator= that is doing something wacky?

Also, your code worked fine for me with both g++ 4.3.2 on Linux and g++ 4.2.1 on Mac (after I defined getX, getY, converted DebugLog to printf and used float instead of GLfloat).

R Samuel Klatchko
I would also say the same, some code is not visible. From the code displayed above, I cannot see any problem.
jdehaan
Thank you the problem was not in copy constructor, but in the assignment operator.
Ilya
Not exactly accurate. Firstly, it will invoke both becauase of the explicit cast (which *will* invoke copy constructor). Secondly, on top of that it can invoke copy constructor as many times as it wants when initializing the parameter of `operator =`.
AndreyT
+1  A: 

I think the problem is in your assignment operator. How does operator= look like?

It seems that operator= is calling him self in some way. Is it possible that the code snippet is taken from the body of operator= itself?

If it is then the solution is to change the code (inside operator=) such that it uses the copy ctor. The canonical form is as follows:

Vector2f temp = Vector2f(arg);
swap(*this, temp)   // You need to implement a swap method
return *this; 

(See Exceptional C++ by Herb Sutter for more details)

Itay
Hmmm. This answer duplicates R Samuel Klatchko's answer. Accept duplicate answer?
Alexey Malistov
+1  A: 

In the second case you are performing assignment, not construction. You haven't defined your own version of copy-assignment operator, which means that the compiler will provide one for you. The compiler-provided copy-assignment operator will be implicitly declared as

Vector2f& operator =(const Vector2f& rhs);

Note that the only parameter of this operator has a reference-to-const type.

In your code you are insisting on supplying a temporary rvalue object of type Vector2f on the right-hand side of assignment.

tempVector2 = Vector2f(0.0f, 0.0f);

This means that the reference parameter of the operator = is initialized with a temporary rvalue of class type. According to he language specification (8.5.3/5), the compiler is allowed to copy that temporary object as many times as it wants before actually attaching the reference. Eventually it must stop copying and finally call the operator. Normally compiler don't go crazy with the copying (most of them do no copying at all), however in your case this seems to be the issue. For some reason, your compiler gets locked in an infinite copying loop, never stopping it. I don't know what is causing this. Could be a bug in the compiler.

Note that even in the

tempVector2 = Vector2f(tempVector1);  

you are still supplying the right-hand side in form of a temporary rvalue. The rvalue is the result of the explicit cast to Vector2f you for some reason put there. I have no idea why you did this. If the problem is indeed in the compiler (and not in the code you are not showing us), I'm sure that if you do just

tempVector2 = tempVector1;  

the assignment will be carried out without any problems. This is actually how you can work around the issue if it turns out to be a bug in the compiler: stop using temporaries as the arguments to copy-constructor and copy-assignment operator.

AndreyT
A: 

Run it under the IDE. When it's in the loop, press the "Pause" button. You will see exactly what the problem is. Here's why.

Mike Dunlavey