CODEWITHSUNDEEP
c++copy-constructor
Alexandru Barbarosie
Alexandru Barbarosie

Reputation: 3002

Copy constructor invokes infinite loop although having call by reference

The code invoking the loop:

Foo temp = Foo(token.substr(0,i));
this->leftExpression = temp;

Having Foo declared as: 

class Foo{
    private:
        std::string expr;
        std::vector <Bar*> tokens;
        std::vector <std::string> stringtoken;

And the CCTOR invoking the loop:

Foo::Foo(const Foo& a_expr){
    this->expr = a_expr.expr;
    this->tokens = a_expr.tokens;
    this->stringtoken = a_expr.stringtoken;
}

What is invoking this loop?

EDIT:

Assignment operator: 

Foo& Foo::operator=(const Foo& a_expr){
    Foo temp(a_expr);
    std::swap(*this, temp);
    return (*this);
}

Upvotes: 3

Views: 1134

Answers (2)

Mike Seymour
Mike Seymour

Reputation: 254581

The problem is here:

std::swap(*this, temp);

The default implementation of std::swap uses assignment, hence the infinite recursion when calling it from your assignment operator.

If you really need to write your own assignment operator, then write your own swap, for example:

void Foo::swap(Foo & other) {
    using std::swap;
    swap(expr, other.expr);
    // and for all other members
}

Foo& Foo::operator=(Foo temp){
    this->swap(temp);
    return (*this);
}

In this case, it looks like all the members are correctly copyable (although you might need to be careful of the dumb pointers in tokens). If that is the case, then there's no need to write your own destructor, copy constructor or copy-assignment operator at all - just let the implicit ones do the right thing.

Upvotes: 5

Daniel Frey
Daniel Frey

Reputation: 56883

Aaaand there's your problem: The assignment operator creates a copy and calls std::swap, which in turn calls the assignment operator.

You could either implement the assignment operator without using std::swap, or you could implement swap without using the default implementation and instead use the "copy-and-swap"-idiom.

Upvotes: 1

Related Questions