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

Matthias Braun via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 18 14:24:26 PDT 2016


MatzeB accepted this revision.
MatzeB added a comment.
This revision is now accepted and ready to land.

LGTM, but should have a test and should not be committed until you are ready to commit code that is using it.


================
Comment at: include/llvm/ADT/SetVector.h:154
@@ -153,1 +153,3 @@
 
+  /// \brief Erase a single element from the set vector.
+  /// \returns an iterator pointing to the next element that followed the
----------------
If the description fits into 1 line you can leave out the "\brief".

================
Comment at: include/llvm/ADT/SetVector.h:163-165
@@ +162,5 @@
+    assert(VI != vector_.end() && "Iterator to erase is out of bounds.");
+    if (!set_.erase(V)) {
+      assert(false && "Corrupted SetVector instances!");
+    }
+    return vector_.erase(VI);
----------------
It's probably best not to check the return value of set_.erase(). llvm::DensetSet does indeed return a bool for erase(), however we can also use an std::set as a set and that does return an iterator and not a bool.


http://reviews.llvm.org/D18281





More information about the llvm-commits mailing list