[LLVMdev] [Patch] Fix bug in llvm::SmallVectorIml<>::insert

[Michael Spencer]

On Sat, Mar 19, 2011 at 2:26 PM, Johannes Schaub (litb)
<schaub.johannes at googlemail.com> wrote:
> Johannes Schaub (litb) wrote:
>
>> This fixes a bug in SmallVectorImpl<>::insert, which were not behaving
>> correctly on inserting an empty range into an empty vector:
>>
>> #include <llvm/ADT/SmallVector.h>
>> #include <cassert>
>>
>> int main() {
>>   llvm::SmallVector<int, 1> v, w;
>>   llvm::SmallVector<int, 1>::iterator it =
>>     v.insert(v.end(), w.begin(), w.end());
>>   assert(it == v.end());
>> }
>>
>> The insert function(s) would incorrectly return "this->end()-1". I
>> attached the patch which I diff'ed from trunk. Is it important enough to
>> be backported to llvm2.9 ?
>>
>> I would like to ask someone to commit it to wherever it fits. Thanks!
>
> Hmm, I don't understand the rationale of the return value of the range-
> insert and repeated-insert.
>
> The range-insert returns an iterator pointing exactly at the last value
> inserted.
>
> But the repeated-insert (i.e Insert(Position, RepeatCount, Value))
> apparently returns an iterator pointing to the first value inserted. Is this
> actually intended?
>
> My patch is inconsistent with this in mind, because it always returns a
> pointer to the first value inserted. Please don't apply it yet. We need to
> clear this up first and fix my patch.
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>

Well, assuming that SmallVector should be as similar to std::vector as
possible, and looking at n3290 23.2.3/{8,9}:

8 The iterator returned from a.insert(p, n, t) points to the copy of
the first element inserted into a, or p if n == 0.
9 The iterator returned from a.insert(p, i, j) points to the copy of
the first element inserted into a, or p if i == j.

The answer is rather clear.

- Michael Spencer