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

[Daniel Berlin via llvm-commits]

On Thu, Mar 24, 2016 at 11:54 AM, Jun Bum Lim via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> junbuml added inline comments.
>
> ================
> Comment at: include/llvm/ADT/SetVector.h:159-165
> @@ +158,9 @@
> +  iterator erase(iterator I) {
> +    const key_type &V = *I;
> +    typename vector_type::iterator VI =
> +        std::find(vector_.begin(), vector_.end(), V);
> +    assert(VI != vector_.end() && "Iterator to erase is out of bounds.");
> +    assert(set_.count(V) && "Corrupted SetVector instances!");
> +    set_.erase(V);
> +    return vector_.erase(VI);
> +  }
> ----------------
> dblaikie wrote:
> > This seems like it's doing extra work to find the element in the vector,
> given an iterator in the vector to begin with - right?
> >
> > Should just be:
> >
> >   set_.erase(V);
> >   return vector_.erase(I);
> >
> > or am I missing something? (don't mind if you keep the
> assert(set_.count(V)))
> We cannot use the given iterator I directly because it's const_iterator
> which cannot be used in vector_.erase(). Please let me know if there is any
> better way to get a non const iterator which we can pass to vector_.erase().
>

Errr, it should work for C++11, as the erase member will take a
const_iterator.
If not, the range version should *definitely work* IE vector_.erase(I, I).


>
>
> http://reviews.llvm.org/D18281
>
>
>
> _______________________________________________
> 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/20160324/8fd6638c/attachment.html>