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

[Michael Spencer]

On Thu, Oct 21, 2010 at 1:58 PM, Bob Floyd <bobfloyd at comcast.net> wrote:
> 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.

> Bob
>
> -----Original Message-----
> From: Michael Spencer [mailto:bigcheesegs at gmail.com]
> Sent: Thursday, October 21, 2010 7:53 AM
> To: Bob Floyd
> Cc: llvmdev at cs.uiuc.edu
> Subject: Re: [LLVMdev] MS VS2010 std implementation: "Cannot assign iterators to two different blocks!"
>
> On Mon, Oct 4, 2010 at 9:52 PM, Bob Floyd <bobfloyd at comcast.net> wrote:
>> 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<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
>
>