[llvm] r279484 - ADT: Separate some list manipulation API into ilist_base, NFC
Duncan P. N. Exon Smith via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 22 15:21:07 PDT 2016
Author: dexonsmith
Date: Mon Aug 22 17:21:07 2016
New Revision: 279484
URL: http://llvm.org/viewvc/llvm-project?rev=279484&view=rev
Log:
ADT: Separate some list manipulation API into ilist_base, NFC
Separate algorithms in iplist<T> that don't depend on T into ilist_base,
and unit test them.
While I was adding unit tests for these algorithms anyway, I also added
unit tests for ilist_node_base and ilist_sentinel<T>.
To make the algorithms and unit tests easier to write, I also did the
following minor changes as a drive-by:
- encapsulate Prev/Next in ilist_node_base to so that algorithms are
easier to read, and
- update ilist_node_access API to take nodes by reference.
There should be no real functionality change here.
Added:
llvm/trunk/unittests/ADT/IListBaseTest.cpp
llvm/trunk/unittests/ADT/IListNodeBaseTest.cpp
llvm/trunk/unittests/ADT/IListSentinelTest.cpp
Modified:
llvm/trunk/include/llvm/ADT/ilist.h
llvm/trunk/include/llvm/ADT/ilist_node.h
llvm/trunk/unittests/ADT/CMakeLists.txt
Modified: llvm/trunk/include/llvm/ADT/ilist.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/ilist.h?rev=279484&r1=279483&r2=279484&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/ilist.h (original)
+++ llvm/trunk/include/llvm/ADT/ilist.h Mon Aug 22 17:21:07 2016
@@ -50,30 +50,19 @@ struct ilist_node_access {
return N;
}
- template <typename T> static ilist_node<T> *getPrev(ilist_node<T> *N) {
- return N->getPrev();
+ template <typename T> static ilist_node<T> *getPrev(ilist_node<T> &N) {
+ return N.getPrev();
}
- template <typename T> static ilist_node<T> *getNext(ilist_node<T> *N) {
- return N->getNext();
+ template <typename T> static ilist_node<T> *getNext(ilist_node<T> &N) {
+ return N.getNext();
}
- template <typename T> static const ilist_node<T> *getPrev(const ilist_node<T> *N) {
- return N->getPrev();
- }
- template <typename T> static const ilist_node<T> *getNext(const ilist_node<T> *N) {
- return N->getNext();
- }
-
- template <typename T> static void setPrev(ilist_node<T> *N, ilist_node<T> *Prev) {
- N->setPrev(Prev);
- }
- template <typename T> static void setNext(ilist_node<T> *N, ilist_node<T> *Next) {
- N->setNext(Next);
- }
- template <typename T> static void setPrev(ilist_node<T> *N, std::nullptr_t) {
- N->setPrev(nullptr);
- }
- template <typename T> static void setNext(ilist_node<T> *N, std::nullptr_t) {
- N->setNext(nullptr);
+ template <typename T>
+ static const ilist_node<T> *getPrev(const ilist_node<T> &N) {
+ return N.getPrev();
+ }
+ template <typename T>
+ static const ilist_node<T> *getNext(const ilist_node<T> &N) {
+ return N.getNext();
}
};
@@ -228,12 +217,12 @@ public:
// Increment and decrement operators...
ilist_iterator &operator--() {
- NodePtr = ilist_node_access::getPrev(NodePtr);
+ NodePtr = ilist_node_access::getPrev(*NodePtr);
assert(NodePtr && "--'d off the beginning of an ilist!");
return *this;
}
ilist_iterator &operator++() {
- NodePtr = ilist_node_access::getNext(NodePtr);
+ NodePtr = ilist_node_access::getNext(*NodePtr);
return *this;
}
ilist_iterator operator--(int) {
@@ -271,6 +260,64 @@ template<typename NodeTy> struct simplif
}
};
+/// Implementations of list algorithms using ilist_node_base.
+class ilist_base {
+public:
+ static void insertBeforeImpl(ilist_node_base &Next, ilist_node_base &N) {
+ ilist_node_base &Prev = *Next.getPrev();
+ N.setNext(&Next);
+ N.setPrev(&Prev);
+ Prev.setNext(&N);
+ Next.setPrev(&N);
+ }
+
+ static void removeImpl(ilist_node_base &N) {
+ ilist_node_base *Prev = N.getPrev();
+ ilist_node_base *Next = N.getNext();
+ Next->setPrev(Prev);
+ Prev->setNext(Next);
+
+ // Not strictly necessary, but helps catch a class of bugs.
+ N.setPrev(nullptr);
+ N.setNext(nullptr);
+ }
+
+ static void transferBeforeImpl(ilist_node_base &Next, ilist_node_base &First,
+ ilist_node_base &Last) {
+ assert(&Next != &Last && "Should be checked by callers");
+ assert(&First != &Last && "Should be checked by callers");
+ // Position cannot be contained in the range to be transferred.
+ assert(&Next != &First &&
+ // Check for the most common mistake.
+ "Insertion point can't be one of the transferred nodes");
+
+ ilist_node_base &Final = *Last.getPrev();
+
+ // Detach from old list/position.
+ First.getPrev()->setNext(&Last);
+ Last.setPrev(First.getPrev());
+
+ // Splice [First, Final] into its new list/position.
+ ilist_node_base &Prev = *Next.getPrev();
+ Final.setNext(&Next);
+ First.setPrev(&Prev);
+ Prev.setNext(&First);
+ Next.setPrev(&Final);
+ }
+
+ template <class T>
+ static void insertBefore(ilist_node<T> &Next, ilist_node<T> &N) {
+ insertBeforeImpl(Next, N);
+ }
+
+ template <class T> static void remove(ilist_node<T> &N) { removeImpl(N); }
+
+ template <class T>
+ static void transferBefore(ilist_node<T> &Next, ilist_node<T> &First,
+ ilist_node<T> &Last) {
+ transferBeforeImpl(Next, First, Last);
+ }
+};
//===----------------------------------------------------------------------===//
//
@@ -280,7 +327,7 @@ template<typename NodeTy> struct simplif
/// ilist_sentinel, which holds pointers to the first and last nodes in the
/// list.
template <typename NodeTy, typename Traits = ilist_traits<NodeTy>>
-class iplist : public Traits, ilist_node_access {
+class iplist : public Traits, ilist_base, ilist_node_access {
// TODO: Drop this assertion and the transitive type traits anytime after
// v4.0 is branched (i.e,. keep them for one release to help out-of-tree code
// update).
@@ -356,13 +403,7 @@ public:
}
iterator insert(iterator where, NodeTy *New) {
- node_type *NewN = this->getNodePtr(New);
- node_type *Next = where.getNodePtr();
- node_type *Prev = this->getPrev(Next);
- this->setNext(NewN, Next);
- this->setPrev(NewN, Prev);
- this->setNext(Prev, NewN);
- this->setPrev(Next, NewN);
+ ilist_base::insertBefore(*where.getNodePtr(), *this->getNodePtr(New));
this->addNodeToList(New); // Notify traits that we added a node...
return iterator(New);
@@ -381,24 +422,9 @@ public:
NodeTy *remove(iterator &IT) {
assert(IT != end() && "Cannot remove end of list!");
- NodeTy *Node = &*IT;
- node_type *Base = this->getNodePtr(Node);
- node_type *Next = this->getNext(Base);
- node_type *Prev = this->getPrev(Base);
-
- this->setNext(Prev, Next);
- this->setPrev(Next, Prev);
- IT = iterator(*Next);
+ NodeTy *Node = &*IT++;
+ ilist_base::remove(*this->getNodePtr(Node));
this->removeNodeFromList(Node); // Notify traits that we removed a node...
-
- // Set the next/prev pointers of the current node to null. This isn't
- // strictly required, but this catches errors where a node is removed from
- // an ilist (and potentially deleted) with iterators still pointing at it.
- // After those iterators are incremented or decremented, they become
- // default-constructed iterators, and will assert on increment, decrement,
- // and dereference instead of "usually working".
- this->setNext(Base, nullptr);
- this->setPrev(Base, nullptr);
return Node;
}
@@ -431,32 +457,11 @@ private:
// [first, last) into position.
//
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.
- assert(position != first &&
- // Check for the most common mistake.
- "Insertion point can't be one of the transferred nodes");
-
if (position == last)
return;
- // Get raw hooks to the first and final nodes being transferred.
- node_type *First = first.getNodePtr();
- node_type *Final = (--last).getNodePtr();
-
- // Detach from old list/position.
- node_type *Prev = this->getPrev(First);
- node_type *Next = this->getNext(Final);
- this->setNext(Prev, Next);
- this->setPrev(Next, Prev);
-
- // Splice [First, Final] into its new list/position.
- Next = position.getNodePtr();
- Prev = this->getPrev(Next);
- this->setNext(Final, Next);
- this->setPrev(First, Prev);
- this->setNext(Prev, First);
- this->setPrev(Next, Final);
+ ilist_base::transferBefore(*position.getNodePtr(), *first.getNodePtr(),
+ *last.getNodePtr());
// Callback. Note that the nodes have moved from before-last to
// before-position.
Modified: llvm/trunk/include/llvm/ADT/ilist_node.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/ilist_node.h?rev=279484&r1=279483&r2=279484&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/ilist_node.h (original)
+++ llvm/trunk/include/llvm/ADT/ilist_node.h Mon Aug 22 17:21:07 2016
@@ -23,18 +23,22 @@ template<typename NodeTy>
struct ilist_traits;
/// Base class for ilist nodes.
-struct ilist_node_base {
+class ilist_node_base {
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
PointerIntPair<ilist_node_base *, 1> PrevAndSentinel;
+#else
+ ilist_node_base *Prev = nullptr;
+#endif
+ ilist_node_base *Next = nullptr;
+public:
+#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
void setPrev(ilist_node_base *Prev) { PrevAndSentinel.setPointer(Prev); }
ilist_node_base *getPrev() const { return PrevAndSentinel.getPointer(); }
bool isKnownSentinel() const { return PrevAndSentinel.getInt(); }
void initializeSentinel() { PrevAndSentinel.setInt(true); }
#else
- ilist_node_base *Prev = nullptr;
-
void setPrev(ilist_node_base *Prev) { this->Prev = Prev; }
ilist_node_base *getPrev() const { return Prev; }
@@ -42,7 +46,8 @@ struct ilist_node_base {
void initializeSentinel() {}
#endif
- ilist_node_base *Next = nullptr;
+ void setNext(ilist_node_base *Next) { this->Next = Next; }
+ ilist_node_base *getNext() const { return Next; }
};
struct ilist_node_access;
@@ -51,6 +56,7 @@ template <typename NodeTy> class ilist_s
/// Templated wrapper class.
template <typename NodeTy> class ilist_node : ilist_node_base {
+ friend class ilist_base;
friend struct ilist_node_access;
friend struct ilist_traits<NodeTy>;
friend class ilist_iterator<NodeTy>;
@@ -63,15 +69,19 @@ private:
ilist_node *getPrev() {
return static_cast<ilist_node *>(ilist_node_base::getPrev());
}
- ilist_node *getNext() { return static_cast<ilist_node *>(Next); }
+ ilist_node *getNext() {
+ return static_cast<ilist_node *>(ilist_node_base::getNext());
+ }
const ilist_node *getPrev() const {
return static_cast<ilist_node *>(ilist_node_base::getPrev());
}
- const ilist_node *getNext() const { return static_cast<ilist_node *>(Next); }
+ const ilist_node *getNext() const {
+ return static_cast<ilist_node *>(ilist_node_base::getNext());
+ }
void setPrev(ilist_node *N) { ilist_node_base::setPrev(N); }
- void setNext(ilist_node *N) { Next = N; }
+ void setNext(ilist_node *N) { ilist_node_base::setNext(N); }
public:
ilist_iterator<NodeTy> getIterator() { return ilist_iterator<NodeTy>(*this); }
Modified: llvm/trunk/unittests/ADT/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/CMakeLists.txt?rev=279484&r1=279483&r2=279484&view=diff
==============================================================================
--- llvm/trunk/unittests/ADT/CMakeLists.txt (original)
+++ llvm/trunk/unittests/ADT/CMakeLists.txt Mon Aug 22 17:21:07 2016
@@ -18,6 +18,9 @@ set(ADTSources
FunctionRefTest.cpp
HashingTest.cpp
ilistTest.cpp
+ IListBaseTest.cpp
+ IListNodeBaseTest.cpp
+ IListSentinelTest.cpp
ImmutableMapTest.cpp
ImmutableSetTest.cpp
IntEqClassesTest.cpp
Added: llvm/trunk/unittests/ADT/IListBaseTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/IListBaseTest.cpp?rev=279484&view=auto
==============================================================================
--- llvm/trunk/unittests/ADT/IListBaseTest.cpp (added)
+++ llvm/trunk/unittests/ADT/IListBaseTest.cpp Mon Aug 22 17:21:07 2016
@@ -0,0 +1,102 @@
+//===- unittests/ADT/IListBaseTest.cpp - ilist_base unit tests ------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/ilist.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+
+TEST(IListBaseTest, insertBeforeImpl) {
+ ilist_node_base S, A, B;
+ // [S] <-> [S]
+ S.setPrev(&S);
+ S.setNext(&S);
+
+ // [S] <-> A <-> [S]
+ ilist_base::insertBeforeImpl(S, A);
+ EXPECT_EQ(&A, S.getPrev());
+ EXPECT_EQ(&S, A.getPrev());
+ EXPECT_EQ(&A, S.getNext());
+ EXPECT_EQ(&S, A.getNext());
+
+ // [S] <-> A <-> B <-> [S]
+ ilist_base::insertBeforeImpl(S, B);
+ EXPECT_EQ(&B, S.getPrev());
+ EXPECT_EQ(&A, B.getPrev());
+ EXPECT_EQ(&S, A.getPrev());
+ EXPECT_EQ(&A, S.getNext());
+ EXPECT_EQ(&B, A.getNext());
+ EXPECT_EQ(&S, B.getNext());
+}
+
+TEST(IListBaseTest, removeImpl) {
+ ilist_node_base S, A, B;
+
+ // [S] <-> A <-> B <-> [S]
+ S.setPrev(&S);
+ S.setNext(&S);
+ ilist_base::insertBeforeImpl(S, A);
+ ilist_base::insertBeforeImpl(S, B);
+
+ // [S] <-> B <-> [S]
+ ilist_base::removeImpl(A);
+ EXPECT_EQ(&B, S.getPrev());
+ EXPECT_EQ(&S, B.getPrev());
+ EXPECT_EQ(&B, S.getNext());
+ EXPECT_EQ(&S, B.getNext());
+ EXPECT_EQ(nullptr, A.getPrev());
+ EXPECT_EQ(nullptr, A.getNext());
+
+ // [S] <-> [S]
+ ilist_base::removeImpl(B);
+ EXPECT_EQ(&S, S.getPrev());
+ EXPECT_EQ(&S, S.getNext());
+ EXPECT_EQ(nullptr, B.getPrev());
+ EXPECT_EQ(nullptr, B.getNext());
+}
+
+TEST(IListBaseTest, transferBeforeImpl) {
+ ilist_node_base S1, S2, A, B, C, D, E;
+
+ // [S1] <-> A <-> B <-> C <-> [S1]
+ S1.setPrev(&S1);
+ S1.setNext(&S1);
+ ilist_base::insertBeforeImpl(S1, A);
+ ilist_base::insertBeforeImpl(S1, B);
+ ilist_base::insertBeforeImpl(S1, C);
+
+ // [S2] <-> D <-> E <-> [S2]
+ S2.setPrev(&S2);
+ S2.setNext(&S2);
+ ilist_base::insertBeforeImpl(S2, D);
+ ilist_base::insertBeforeImpl(S2, E);
+
+ // [S1] <-> C <-> [S1]
+ ilist_base::transferBeforeImpl(D, A, C);
+ EXPECT_EQ(&C, S1.getPrev());
+ EXPECT_EQ(&S1, C.getPrev());
+ EXPECT_EQ(&C, S1.getNext());
+ EXPECT_EQ(&S1, C.getNext());
+
+ // [S2] <-> A <-> B <-> D <-> E <-> [S2]
+ EXPECT_EQ(&E, S2.getPrev());
+ EXPECT_EQ(&D, E.getPrev());
+ EXPECT_EQ(&B, D.getPrev());
+ EXPECT_EQ(&A, B.getPrev());
+ EXPECT_EQ(&S2, A.getPrev());
+ EXPECT_EQ(&A, S2.getNext());
+ EXPECT_EQ(&B, A.getNext());
+ EXPECT_EQ(&D, B.getNext());
+ EXPECT_EQ(&E, D.getNext());
+ EXPECT_EQ(&S2, E.getNext());
+}
+
+} // end namespace
Added: llvm/trunk/unittests/ADT/IListNodeBaseTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/IListNodeBaseTest.cpp?rev=279484&view=auto
==============================================================================
--- llvm/trunk/unittests/ADT/IListNodeBaseTest.cpp (added)
+++ llvm/trunk/unittests/ADT/IListNodeBaseTest.cpp Mon Aug 22 17:21:07 2016
@@ -0,0 +1,60 @@
+//===- unittests/ADT/IListNodeBaseTest.cpp - ilist_node_base unit tests ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/ilist_node.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+
+TEST(IListNodeBaseTest, DefaultConstructor) {
+ ilist_node_base A;
+ EXPECT_EQ(nullptr, A.getPrev());
+ EXPECT_EQ(nullptr, A.getNext());
+ EXPECT_FALSE(A.isKnownSentinel());
+}
+
+TEST(IListNodeBaseTest, setPrevAndNext) {
+ ilist_node_base A, B, C;
+ A.setPrev(&B);
+ EXPECT_EQ(&B, A.getPrev());
+ EXPECT_EQ(nullptr, A.getNext());
+ EXPECT_EQ(nullptr, B.getPrev());
+ EXPECT_EQ(nullptr, B.getNext());
+ EXPECT_EQ(nullptr, C.getPrev());
+ EXPECT_EQ(nullptr, C.getNext());
+
+ A.setNext(&C);
+ EXPECT_EQ(&B, A.getPrev());
+ EXPECT_EQ(&C, A.getNext());
+ EXPECT_EQ(nullptr, B.getPrev());
+ EXPECT_EQ(nullptr, B.getNext());
+ EXPECT_EQ(nullptr, C.getPrev());
+ EXPECT_EQ(nullptr, C.getNext());
+}
+
+TEST(IListNodeBaseTest, isKnownSentinel) {
+ ilist_node_base A, B;
+ EXPECT_FALSE(A.isKnownSentinel());
+ A.setPrev(&B);
+ A.setNext(&B);
+ EXPECT_EQ(&B, A.getPrev());
+ EXPECT_EQ(&B, A.getNext());
+ A.initializeSentinel();
+#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
+ EXPECT_TRUE(A.isKnownSentinel());
+#else
+ EXPECT_FALSE(A.isKnownSentinel());
+#endif
+ EXPECT_EQ(&B, A.getPrev());
+ EXPECT_EQ(&B, A.getNext());
+}
+
+} // end namespace
Added: llvm/trunk/unittests/ADT/IListSentinelTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/IListSentinelTest.cpp?rev=279484&view=auto
==============================================================================
--- llvm/trunk/unittests/ADT/IListSentinelTest.cpp (added)
+++ llvm/trunk/unittests/ADT/IListSentinelTest.cpp Mon Aug 22 17:21:07 2016
@@ -0,0 +1,37 @@
+//===- unittests/ADT/IListSentinelTest.cpp - ilist_sentinel unit tests ----===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/ilist.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+
+class Node : public ilist_node<Node> {};
+
+TEST(IListSentinelTest, DefaultConstructor) {
+ ilist_sentinel<Node> S;
+ EXPECT_EQ(&S, ilist_node_access::getPrev(S));
+ EXPECT_EQ(&S, ilist_node_access::getNext(S));
+#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
+ EXPECT_TRUE(S.isKnownSentinel());
+#else
+ EXPECT_FALSE(S.isKnownSentinel());
+#endif
+}
+
+TEST(IListSentinelTest, NormalNodeIsNotKnownSentinel) {
+ Node N;
+ EXPECT_EQ(nullptr, ilist_node_access::getPrev(N));
+ EXPECT_EQ(nullptr, ilist_node_access::getNext(N));
+ EXPECT_FALSE(N.isKnownSentinel());
+}
+
+} // end namespace
More information about the llvm-commits
mailing list