[llvm] r279484 - ADT: Separate some list manipulation API into ilist_base, NFC
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 23 00:55:06 PDT 2016
Build error here:
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/24265/steps/build%20release%20tsan%20with%20clang/logs/stdio
On Mon, Aug 22, 2016 at 3:29 PM Duncan P. N. Exon Smith via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> 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
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160823/e8c0b1bd/attachment.html>
More information about the llvm-commits
mailing list