MS VS2010 std implementation: "Cannot assign iterators to two different blocks!"

When using MS VS2010 there is an issue with std:

SuccIterator implements a partial assignment operator:

inline const _Self &operator=(const _Self &I) {

assert(Term == I.Term &&“Cannot assign iterators to two different blocks!”);

idx = I.idx;

return *this;

}

For copy construction, MS VS2010 std reserves the right, and sometimes calls,

a method to reverse elements which does so by swap:

template inline

void swap(_Ty& _Left, _Ty& _Right)

{ // exchange values stored at _Left and _Right

_Ty _Tmp = _Move(_Left);

_Left = _Move(_Right); <<<<<<<<<<<<< Needs FULL assignment operator!!!

_Right = _Move(_Tmp);

}

When _Ty is SuccIterator const the assert “Cannot assign iterators to two different blocks!” can fail.

Can this be a workaround:

inline const _Self &operator=(const _Self &I) {

new (this) _Self(I);

return *this;

}

The larger question is can both the assert be preserved and swap satisfied?

Hi Bob, was this issue resolved?

Ciao,

Duncan.

Duncan,

As I am both an LLVM user and new to it I do not check in any changes, so I
have not made a fix.

However, to make LLVM work under VS2010 in my environment I replaced the
assignment constructor with the following
which fixes the issue on the simple tests in my environment which I have
tried. Any developer can validate this and check it in, although this fix
deletes the assert check and is not particularly pretty to my taste.

Bob

In CFG.h, replace the assignment constructor with:

////////////////////////////////////////////////////////////////////////////
//////////
  inline const _Self &operator=(const _Self &I) {
    this->~SuccIterator();
    new (this) _Self(I);
    // VS2010 std implements deque (on which stack is built) copy
constructor
    // as reserving the right to swap elements. The swap method uses
`operator=`
    // as follows:
    // template<class _Ty> inline
    // void swap(_Ty& _Left, _Ty& _Right)
    // { // exchange values stored at _Left and _Right
    // _Ty _Tmp = _Move(_Left);
    // _Left = _Move(_Right); <<<<<<<<< calls this `operator=`
    // _Right = _Move(_Tmp);
    // }
    // However, `_Self`, does not implement a full assignment operator
    // in favor of defense against misuse.
#ifdef PROBLEM
    assert(Term == I.Term &&"Cannot assign iterators to two different
blocks!");
    idx = I.idx;
#endif // PROBLEM
    return *this;
  }
////////////////////////////////////////////////////////////////////////////
/////////////

Behalf Of Duncan Sands

When using MS VS2010 there is an issue with std:

`SuccIterator` implements a partial assignment operator:

inline const _Self &operator=(const _Self &I) {

assert\(Term == I\.Term &amp;&amp;&quot;Cannot assign iterators to two different

blocks!");

idx = I\.idx;

return \*this;

\}

For copy construction, MS VS2010 std reserves the right, and sometimes
calls,

a method to reverse elements which does so by swap:

template<class _Ty> inline

void swap(_Ty& _Left, _Ty& _Right)

{ // exchange values stored at _Left and _Right

_Ty _Tmp = _Move(_Left);

This uses the compiler generated default copy constructor which does
an element by element copy.

_Left = _Move(_Right); <<<<<<<<<<<<< Needs FULL assignment operator!!!!

Given that SuccIterator does not implement move semantics, we can
basically remove all the _Move's here, they just turn into
assignments. Thus _Left and _Right are both fully constructed.
_Left.Term == _Right.Term is true if it was true before swap.

_Right = _Move(_Tmp);

}

When `_Ty` is `SuccIterator const` the assert "Cannot assign iterators to
two different blocks!" can fail.

Can this be a workaround:

inline const _Self &operator=(const _Self &I) {

new (this) _Self(I);

return *this;

}

The larger question is can both the assert be preserved and swap satisfied?

I believe this is an error in your code. Are you 100% sure that
_Left.Term == _Right.Term before swap is called?

- Michael Spencer

Michael,

Yes, while stepping through this VS2010 `std::swap` code, `_Move` did nothing, e.g.

////////////////////////////////////////////////////
void swap(_Ty& _Left, _Ty& _Right)
{ // exchange values stored at _Left and _Right
  _Ty _Tmp = _Move(_Left);
  _Left = _Move(_Right);
  _Right = _Move(_Tmp);
}
////////////////////////////////////////////////////

was equivalent to:

////////////////////////////////////////////////////
void swap(_Ty& _Left, _Ty& _Right)
{ // exchange values stored at _Left and _Right
  _Ty _Tmp = Left;
  _Left = _Right;
  _Right = Tmp;
}
////////////////////////////////////////////////////

the deque being reversed had 3 items: `a1`, `a2`, and `a3` (`SuccIterator&` types)

the 1st call to swap `_Left` was `a1` and `_Right` was `a3`

the assignment: "_Left = _Right" was "a1 = a3"

`a1` and `a3` had different `SuccIterator::Term`s, causing the assert in `SuccIterator::operator=` to fail.

This all happened when an llvm stack was being copy constructed (right now I can't be more specific since I'm in-between running code). Finding the microsoft std library doing a reverse during a simple stack copy was a surprise. But on the other hand, since the stack was implemented using deque, it appeared the ms std code was being clever with it.

As I recall, I was not surprised that the `SuccIterator::Term`s in the llvm stack were different. Are you saying
the `Term`s in the llvm stack should have been the same, meaning, perhaps, I have used the API incorrectly? After replacing the `SuccIterator::operator=` I hand-checked the JIT code generated by assembly stepping through it and it was Ok. Suggesting I populated Ok, and different `SuccIterator::Term`s were ok.

Could a solution be to implement `_Move` in `SuccIterator`? I am not familiar with `_Move` semantics in MS std library.

Bob

Michael,

Yes, while stepping through this VS2010 `std::swap` code, `_Move` did nothing, e.g.

////////////////////////////////////////////////////
void swap(_Ty& _Left, _Ty& _Right)
{ // exchange values stored at _Left and _Right
_Ty _Tmp = _Move(_Left);
_Left = _Move(_Right);
_Right = _Move(_Tmp);
}
////////////////////////////////////////////////////

was equivalent to:

////////////////////////////////////////////////////
void swap(_Ty& _Left, _Ty& _Right)
{ // exchange values stored at _Left and _Right
_Ty _Tmp = Left;
_Left = _Right;
_Right = Tmp;
}
////////////////////////////////////////////////////

the deque being reversed had 3 items: `a1`, `a2`, and `a3` (`SuccIterator&` types)

the 1st call to swap `_Left` was `a1` and `_Right` was `a3`

the assignment: "_Left = _Right" was "a1 = a3"

`a1` and `a3` had different `SuccIterator::Term`s, causing the assert in `SuccIterator::operator=` to fail.

Ah, I was assuming a set where Term was equal. I've never used
SuccIterator so I don't know if that is legal.

This all happened when an llvm stack was being copy constructed (right now I can't be more specific since I'm in-between running code). Finding the microsoft std library doing a reverse during a simple stack copy was a surprise. But on the other hand, since the stack was implemented using deque, it appeared the ms std code was being clever with it.

As I recall, I was not surprised that the `SuccIterator::Term`s in the llvm stack were different. Are you saying
the `Term`s in the llvm stack should have been the same, meaning, perhaps, I have used the API incorrectly? After replacing the `SuccIterator::operator=` I hand-checked the JIT code generated by assembly stepping through it and it was Ok. Suggesting I populated Ok, and different `SuccIterator::Term`s were ok.

Could a solution be to implement `_Move` in `SuccIterator`? I am not familiar with `_Move` semantics in MS std library.

_Move is very implementation specific. If you should be able to swap
SuccIterators with different Term's, then the assert needs to be
removed.