views:

70

answers:

3

I have an error (vector iterator incompatibles) during execution in my C++ program that I do not understand. [ (Windows / Visual C++ 2008 Express) ]

Here is a simplified version of my problem :

#include <vector>

class A
{
    int mySuperInt;
public:
    A(int val) : mySuperInt(val) {}
};
class B
{
    std::vector<A*> myAs;
    public:
        B() 
        {
            myAs.push_back(new A(1));
        };
        const std::vector<A*> getA() const {return myAs;}
};

int main()
{
    std::vector<B>* myBs = new std::vector<B>;

    myBs->push_back(B());

    std::vector<B>::const_iterator it_B = myBs->begin();
    for ( ; it_B != myBs->end(); ++it_B)
    {
        std::vector<A*>::const_iterator it_A = it_B->getA().begin();
        for ( ; it_A != it_B->getA().end(); ++it_A) // <-- Error during execution: vector iterator incompatibles
        {
            // Do stuff
            // ...
        }
    }
}

Did I missed something ?

Thanks in advance for your answers.

+6  A: 

Your getA() function returns a vector by value. You're initializing your loop iterator to the beginning of that vector, but since the returned vector is temporary, it is destroyed at the end of that line.

// at the end of this line the vector returned by getA is gone, so it_A is invalid.
std::vector<A*>::const_iterator it_A = it_B->getA().begin();

Therefore the iterator is no longer valid. You should instead return a reference to the vector like this (note the &):

const std::vector<A*> & getA() const {return myAs;}
JoshD
That was... kind of obvious. Thanks a lot JoshD !
Xavier V.
+1  A: 

You realize that B::getAs() returns a copy of its myAs. So the vector that it_A ranges over is a different copy than the one whose getA().end() it keeps being compared to... I.e. the for loop never stops. (Even worse, it_A ranges over a vector<> that is a temporary, so it's destroyed and all kinds of random crap could be written over that storage while you're pretending to iterate through it.)

Eric Towers
+1  A: 

Thanks for the complete and simple repro. The problem here is that you're using iterators from 2 different vectors, this is a runtime debug check.

You probably didn't intend to do this, but it is a result of the return type of getA.

You're returning a copy of the vector and your probably meant to return a reference to the vector, like this:

const std::vector<A*>& getA() const {return myAs;}

Rick