Add an assertion for a likely ilist::splice() contract violation.

The single-element ilist::splice() function supports a noop move:

  List.splice(I, List, I);

The corresponding std::list function doesn't allow that, so add a unit
test to document that behavior.

This also means that

  List.splice(I, List, F);

is somewhat surprisingly not equivalent to

  List.splice(I, List, F, next(F));

This patch adds an assertion to catch the illegal case I == F above.
Alternatively, we could make I == F a legal noop, but that would make
ilist differ even more from std::list.

llvm-svn: 170443

diff --git a/llvm/include/llvm/ADT/ilist.h b/llvm/include/llvm/ADT/ilist.h
index 7f5cd17..36650d4 100644 (file)
--- a/llvm/include/llvm/ADT/ilist.h
+++ b/llvm/include/llvm/ADT/ilist.h
@@ -472,6 +472,10 @@ private:
   //
   void transfer(iterator position, iplist &L2, iterator first, iterator last) {
     assert(first != last && "Should be checked by callers");
+    // Position cannot be contained in the range to be transferred.
+    // Check for the most common mistake.
+    assert(position != first &&
+           "Insertion point can't be one of the transferred nodes");
 
     if (position != last) {
       // Note: we have to be careful about the case when we move the first node

diff --git a/llvm/unittests/ADT/ilistTest.cpp b/llvm/unittests/ADT/ilistTest.cpp
index 83eaa31..711192e 100644 (file)
--- a/llvm/unittests/ADT/ilistTest.cpp
+++ b/llvm/unittests/ADT/ilistTest.cpp
@@ -9,6 +9,7 @@
 
 #include "llvm/ADT/ilist.h"
 #include "llvm/ADT/ilist_node.h"
+#include "llvm/ADT/STLExtras.h"
 #include "gtest/gtest.h"
 #include <ostream>
 
@@ -41,4 +42,24 @@ TEST(ilistTest, Basic) {
   EXPECT_EQ(1, ConstList.back().getPrevNode()->Value);
 }
 
+TEST(ilistTest, SpliceOne) {
+  ilist<Node> List;
+  List.push_back(1);
+
+  // The single-element splice operation supports noops.
+  List.splice(List.begin(), List, List.begin());
+  EXPECT_EQ(1u, List.size());
+  EXPECT_EQ(1, List.front().Value);
+  EXPECT_TRUE(llvm::next(List.begin()) == List.end());
+
+  // Altenative noop. Move the first element behind itself.
+  List.push_back(2);
+  List.push_back(3);
+  List.splice(llvm::next(List.begin()), List, List.begin());
+  EXPECT_EQ(3u, List.size());
+  EXPECT_EQ(1, List.front().Value);
+  EXPECT_EQ(2, llvm::next(List.begin())->Value);
+  EXPECT_EQ(3, List.back().Value);
+}
+
 }