From b8d29bf2e41daa0800c9922b1eb59f8ac2efe192 Mon Sep 17 00:00:00 2001
From: Jakob Stoklund Olesen <stoklund@2pi.dk>
Date: Tue, 18 Dec 2012 19:28:37 +0000
Subject: [PATCH] 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
---
 llvm/include/llvm/ADT/ilist.h    |  4 ++++
 llvm/unittests/ADT/ilistTest.cpp | 21 +++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/llvm/include/llvm/ADT/ilist.h b/llvm/include/llvm/ADT/ilist.h
index 7f5cd1718142..36650d43750c 100644
--- 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 83eaa31981db..711192ed89e7 100644
--- 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);
+}
+
 }