[PATCH] D18281: [SetVector] Add erase() method

[David Blaikie via llvm-commits]

On Fri, Mar 25, 2016 at 11:46 AM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Fri, Mar 25, 2016 at 11:42 AM, Jun Bum Lim via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> junbuml removed rL LLVM as the repository for this revision.
>> junbuml updated this revision to Diff 51666.
>> junbuml added a comment.
>>
>> To avoid the builedbot failure, pass non-const iterator to
>> vector_.erase().
>>
>>
>> http://reviews.llvm.org/D18281
>>
>> Files:
>>   include/llvm/ADT/SetVector.h
>>   unittests/ADT/CMakeLists.txt
>>   unittests/ADT/SetVectorTest.cpp
>>
>> Index: unittests/ADT/SetVectorTest.cpp
>> ===================================================================
>> --- /dev/null
>> +++ unittests/ADT/SetVectorTest.cpp
>> @@ -0,0 +1,34 @@
>> +//===- llvm/unittest/ADT/SetVector.cpp
>> ------------------------------===//
>> +//
>> +//                     The LLVM Compiler Infrastructure
>> +//
>> +// This file is distributed under the University of Illinois Open Source
>> +// License. See LICENSE.TXT for details.
>> +//
>>
>> +//===----------------------------------------------------------------------===//
>> +//
>> +// SetVector unit tests.
>> +//
>>
>> +//===----------------------------------------------------------------------===//
>> +
>> +#include "llvm/ADT/SetVector.h"
>> +#include "gtest/gtest.h"
>> +
>> +using namespace llvm;
>> +
>> +TEST(SetVector, EraseTest) {
>> +  SetVector<int> S;
>> +  S.insert(0);
>> +  S.insert(1);
>> +  S.insert(2);
>> +
>> +  auto I = S.erase(std::next(S.begin()));
>> +
>> +  // Test that the returned iterator is the expected one-after-erase
>> +  // and the size/contents is the expected sequence {0, 2}.
>> +  EXPECT_EQ(std::next(S.begin()), I);
>> +  EXPECT_EQ(2u, S.size());
>> +  EXPECT_EQ(0, *S.begin());
>> +  EXPECT_EQ(2, *std::next(S.begin()));
>> +}
>> +
>> Index: unittests/ADT/CMakeLists.txt
>> ===================================================================
>> --- unittests/ADT/CMakeLists.txt
>> +++ unittests/ADT/CMakeLists.txt
>> @@ -32,6 +32,7 @@
>>    PostOrderIteratorTest.cpp
>>    RangeAdapterTest.cpp
>>    SCCIteratorTest.cpp
>> +  SetVectorTest.cpp
>>    SmallPtrSetTest.cpp
>>    SmallStringTest.cpp
>>    SmallVectorTest.cpp
>> Index: include/llvm/ADT/SetVector.h
>> ===================================================================
>> --- include/llvm/ADT/SetVector.h
>> +++ include/llvm/ADT/SetVector.h
>> @@ -151,6 +151,21 @@
>>      return false;
>>    }
>>
>> +  /// Erase a single element from the set vector.
>> +  /// \returns an iterator pointing to the next element that followed the
>> +  /// element erased. This is the end of the SetVector if the last
>> element is
>> +  /// erased.
>> +  iterator erase(iterator I) {
>> +    const key_type &V = *I;
>> +    assert(set_.count(V) && "Corrupted SetVector instances!");
>> +    set_.erase(V);
>> +    // FIXME: No need to use the non-const iterator when built with
>> +    // std:vector.erase(const_iterator) as defined in C++11.
>> +    typename vector_type::iterator NI (vector_.begin());
>>
>
Probably mention in the comment that this is for compatibility with
non-standard libstdc++ up to 4.8 (fixed in 4.9)


>
> Use = here, rather than () (& probably jsut auto)
>
> auto NI = vector_.begin();
>
>
>> +    std::advance(NI, std::distance<iterator>(NI, I));
>> +    return vector_.erase(NI);
>> +  }
>> +
>>    /// \brief Remove items from the set vector based on a predicate
>> function.
>>    ///
>>    /// This is intended to be equivalent to the following code, if we
>> could
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160325/4df03966/attachment.html>