From: Jakob Stoklund Olesen Date: Tue, 18 Dec 2012 19:28:37 +0000 (+0000) Subject: Add an assertion for a likely ilist::splice() contract violation. X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=7f1d6d688f6ae288a16a4151e8a27b518d97f6f7;p=oota-llvm.git 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. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@170443 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/ADT/ilist.h b/include/llvm/ADT/ilist.h index 7f5cd171814..36650d43750 100644 --- a/include/llvm/ADT/ilist.h +++ b/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/unittests/ADT/ilistTest.cpp b/unittests/ADT/ilistTest.cpp index 83eaa31981d..711192ed89e 100644 --- a/unittests/ADT/ilistTest.cpp +++ b/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 @@ -41,4 +42,24 @@ TEST(ilistTest, Basic) { EXPECT_EQ(1, ConstList.back().getPrevNode()->Value); } +TEST(ilistTest, SpliceOne) { + ilist 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); +} + }