Sign in to follow this  
Sanctux

[SOLVED] Assigning Iterator to Iterator

Recommended Posts

I'm getting a debug assertion "vector iterators incompatible" so I traced it down. I discovered that when I step over in this line
activeItem = menuItems.begin(); // where activeItem is a vector iterator and a member variable of Menu and menuItems is a vector

activeItem has a member (error) with value 0 Then I went over to the constructor (activeItem is a member variable of Menu), activeItem still had a member (error) with value 0. What's up with this? Are iterators required to be initialized or something? [Edited by - Sanctux on December 24, 2009 2:33:42 AM]

Share this post


Link to post
Share on other sites
While you say they are vector iterators, that is not enough to fully specify their type. vector<int>::iterator and vector<double>::iterator are both "vector iterators" but it doesn't mean they are compatible.

Share this post


Link to post
Share on other sites
Thanks for the reply, stonemetal.

Here's what I have:

typedef std::pair<int, MenuItem> MenuPair;
typedef std::vector<MenuPair> MenuVec;
typedef MenuVec::iterator MenuIter;

class Menu
{
// ...
private:
// ...
MenuVec menuItems;
MenuIter activeItem;
// ...
};



I'm pretty sure they're the same type of iterators; unless I got lost in the typedefs. [totally]

Share this post


Link to post
Share on other sites
Quote:
Original post by diablos_blade
Is the function this code is being run in a const member function?

Nope.

Here's the source of the assertion (in vector):
void _Compat(const _Myt& _Right) const
{ // test for compatible iterator pair
if (this->_Mycont == 0 || this->_Mycont != _Right._Mycont)
{
_DEBUG_ERROR("vector iterators incompatible");
_SCL_SECURE_INVALID_ARGUMENT;
}
}


In case it might help, further up the call stack:
	bool operator==(const _Myt& _Right) const
{ // test for iterator equality

#if _HAS_ITERATOR_DEBUGGING
_Compat(_Right);
#else
_SCL_SECURE_VALIDATE(this->_Has_container() && this->_Same_container(_Right));
#endif /* _HAS_ITERATOR_DEBUGGING */

return (_Myptr == _Right._Myptr);
}



Thanks for the replies! [smile]

Share this post


Link to post
Share on other sites
Was one of the iterators assigned to a different container previously?

What is the value of _Mycont for each iterator when it asserts?

Can you check operator= for the iterator to make sure it assigns the _Mycont member?

Share this post


Link to post
Share on other sites
Quote:
Original post by RDragon1
Was one of the iterators assigned to a different container previously?

What is the value of _Mycont for each iterator when it asserts?

Can you check operator= for the iterator to make sure it assigns the _Mycont member?

1. Well, the only time I did anything else with activeItem was here:
activeItem = menuItems.begin();
which is called before the point of assertion. Otherwise, I don't think I've touched it.

2. this->_Mycont has value 0x00000000 and its member _Myfirstiter has CXX0030: Error: expression cannot be evaluated. _Right._Mycont has value 0x056a4844.

3. Here's operator=:
	_Myt& operator=(const _Myt& _Right)
{ // assign _Right
if (this != &_Right)
{ // worth doing

#if _HAS_ITERATOR_DEBUGGING
this->_Orphan_all();
#endif /* _HAS_ITERATOR_DEBUGGING */

if (_Right.size() == 0)
clear(); // new sequence empty, erase existing sequence
else if (_Right.size() <= size())
{ // enough elements, copy new and destroy old
pointer _Ptr = _STDEXT unchecked_copy(_Right._Myfirst, _Right._Mylast,
_Myfirst); // copy new
_Destroy(_Ptr, _Mylast); // destroy old
_Mylast = _Myfirst + _Right.size();
}
else if (_Right.size() <= capacity())
{ // enough room, copy and construct new
pointer _Ptr = _Right._Myfirst + size();
_STDEXT unchecked_copy(_Right._Myfirst, _Ptr, _Myfirst);
_Mylast = _Ucopy(_Ptr, _Right._Mylast, _Mylast);
}
else
{ // not enough room, allocate new array and construct new
if (_Myfirst != 0)
{ // discard old array
_Destroy(_Myfirst, _Mylast);
this->_Alval.deallocate(_Myfirst, _Myend - _Myfirst);
}
if (_Buy(_Right.size()))
_Mylast = _Ucopy(_Right._Myfirst, _Right._Mylast,
_Myfirst);
}
}
return (*this);
}

and
	_Mytype& operator=(const _Mytype& _Right)
{ // assign _Vb_reference _Right to bit
return (*this = bool(_Right));
}

_Mytype& operator=(bool _Val)
{ // assign _Val to bit
if (_Val)
*_Getptr() |= _Mask();
else
*_Getptr() &= ~_Mask();
return (*this);
}


Thanks!

Share this post


Link to post
Share on other sites
Quote:
Original post by RDragon1
Oh, your problem is likely that you called cont.begin() when the container was empty. That will return you a nice invalid iterator


I'm sorry, I do not quite understand. When debugging, menuItems was populated with 2 items, and activeItem simply had (error) as a symbol and 0 as its value, and maintained that state ever since its construction in its parent class.

Share this post


Link to post
Share on other sites
Did it have 2 items when you executed this:

activeItem = menuItems.begin();

?

I suspect it was .empty() at this point

If not, we'll need a full small test case that shows the problem to continue from here

Share this post


Link to post
Share on other sites
Quote:
Original post by RDragon1
Did it have 2 items when you executed this:

activeItem = menuItems.begin();

?

I suspect it was .empty() at this point

If not, we'll need a full small test case that shows the problem to continue from here


Why, yes.


Note the (error) symbol I mentioned.


I then experimented and look at the results:

(again the error)

Share this post


Link to post
Share on other sites
Quote:
Original post by RDragon1
Are you absolutely sure that's always getting hit before that comparison statement gets executed?


Yes.
menu.AddItem(MenuPair(PLAY, MenuItem("Play")));
menu.AddItem(MenuPair(QUIT, MenuItem("Quit")));


is called in the constructor of MainMenuState, and I've stepped through the whole thing many times.

Could it be a bug on my part? I do not quite fully understand the nature of iterators.

Would posting the project help?

Share this post


Link to post
Share on other sites

void Menu::AddItem(MenuPair item)
{
menuItems.push_back(item);
// if (menuItems.size() == 1)
activeItem = menuItems.begin();
}



Try that. Iterators are invalidated when the internals of the container are moved around in memory. Adding an item to a vector for instance will re-allocate memory if the array is too small accommodate the new item, thus invalidating all previously "purchased" iterators, see below for a code sample demonstrating the problem. Possible fixes include reserve-ing some amount of menu items before hand (may cause problems later), revalidating iterators as the code above shows, using a separate container (such as std::list, which I believe does not suffer this problem), or you can simply refer to the activeItem as an index in the array.


int main() {
int* arr = new int[2]; // equivalent to vector::reserve(2)
int arrSize = 0;

arr[0] = 123; // equivalent to vector::push_back( 123 )
arr[1] = 456; // equivalent to vector::push_back( 456 )

int* selection = &arr[1];

// push_back( 789 ):
int* temp = new int[3];
for ( int i = 0; i < 2; ++i )
temp[i] = arr[i];
delete[] arr;
arr = temp;
temp = 0;

arr[2] = 789;

*selection = 1; // Boom!

delete[] arr;
}

Share this post


Link to post
Share on other sites
Quote:
Original post by _fastcall
*** Source Snippet Removed ***

Try that. Iterators are invalidated when the internals of the container are moved around in memory. Adding an item to a vector for instance will re-allocate memory if the array is too small accommodate the new item, thus invalidated all previously "purchased" iterators. Possible fixes include reserve-ing some amount of menu items before hand (may cause problems later), revalidating iterators as the code above shows, using a separate container (such as std::list, which I believe does not suffer this problem), or you can simply refer to the activeItem as an index in the array.


Oh, I see. Revalidating the iterator seems to have fixed the problem. Thanks tons, _fastcall and RDragon1.


++rating; [wink]

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this