[llvm-commits] [llvm] r170443 - in /llvm/trunk: include/llvm/ADT/ilist.h unittests/ADT/ilistTest.cpp

[Jakob Stoklund Olesen]

Author: stoklund
Date: Tue Dec 18 13:28:37 2012
New Revision: 170443

URL: http://llvm.org/viewvc/llvm-project?rev=170443&view=rev
Log:
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.

Modified:
    llvm/trunk/include/llvm/ADT/ilist.h
    llvm/trunk/unittests/ADT/ilistTest.cpp

Modified: llvm/trunk/include/llvm/ADT/ilist.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/ilist.h?rev=170443&r1=170442&r2=170443&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/ilist.h (original)
+++ llvm/trunk/include/llvm/ADT/ilist.h Tue Dec 18 13:28:37 2012
@@ -472,6 +472,10 @@
   //
   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

Modified: llvm/trunk/unittests/ADT/ilistTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/ilistTest.cpp?rev=170443&r1=170442&r2=170443&view=diff
==============================================================================
--- llvm/trunk/unittests/ADT/ilistTest.cpp (original)
+++ llvm/trunk/unittests/ADT/ilistTest.cpp Tue Dec 18 13:28:37 2012
@@ -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 @@
   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);
+}
+
 }