2008-05-29 19:41:17 +02:00
|
|
|
//==-- llvm/ADT/ilist.h - Intrusive Linked List Template ---------*- C++ -*-==//
|
2009-01-08 03:21:23 +01:00
|
|
|
//
|
2003-10-20 21:46:57 +02:00
|
|
|
// The LLVM Compiler Infrastructure
|
|
|
|
//
|
2007-12-29 23:59:10 +01:00
|
|
|
// This file is distributed under the University of Illinois Open Source
|
|
|
|
// License. See LICENSE.TXT for details.
|
2009-01-08 03:21:23 +01:00
|
|
|
//
|
2003-10-20 21:46:57 +02:00
|
|
|
//===----------------------------------------------------------------------===//
|
2002-06-25 18:13:24 +02:00
|
|
|
//
|
|
|
|
// This file defines classes to implement an intrusive doubly linked list class
|
2007-04-17 20:41:42 +02:00
|
|
|
// (i.e. each node of the list must contain a next and previous field for the
|
2002-06-25 18:13:24 +02:00
|
|
|
// list.
|
|
|
|
//
|
2016-08-12 20:14:42 +02:00
|
|
|
// The ilist class itself should be a plug in replacement for list. This list
|
|
|
|
// replacement does not provide a constant time size() method, so be careful to
|
|
|
|
// use empty() when you really want to know if it's empty.
|
2002-06-25 18:13:24 +02:00
|
|
|
//
|
Reapply "ADT: Remove UB in ilist (and use a circular linked list)"
This reverts commit r279053, reapplying r278974 after fixing PR29035
with r279104.
Note that r279312 has been committed in the meantime, and this has been
rebased on top of that. Otherwise it's identical to r278974.
Note for maintainers of out-of-tree code (that I missed in the original
message): if the new isKnownSentinel() assertion is firing from
ilist_iterator<>::operator*(), this patch has identified a bug in your
code. There are a few common patterns:
- Some IR-related APIs htake an IRUnit* that might be nullptr, and pass
in an incremented iterator as an insertion point. Some old code was
using "&*++I", which in the case of end() only worked by fluke. If
the IRUnit in question inherits from ilist_node_with_parent<>, you can
use "I->getNextNode()". Otherwise, use "List.getNextNode(*I)".
- In most other cases, crashes on &*I just need to check for I==end()
before dereferencing.
- There's also occasional code that sends iterators into a function, and
then starts calling I->getOperand() (or other API). Either check for
end() before the entering the function, or early exit.
Note for if the static_assert with HasObsoleteCustomization is firing
for you:
- r278513 has examples of how to stop using custom sentinel traits.
- r278532 removed ilist_nextprev_traits since no one was using it. See
lld's r278469 for the only migration I needed to do.
Original commit message follows.
----
This removes the undefined behaviour (UB) in ilist/ilist_node/etc.,
mainly by removing (gutting) the ilist_sentinel_traits customization
point and canonicalizing on a single, efficient memory layout. This
fixes PR26753.
The new ilist is a doubly-linked circular list.
- ilist_node_base has two ilist_node_base*: Next and Prev. Size-of: two
pointers.
- ilist_node<T> (size-of: two pointers) is a type-safe wrapper around
ilist_node_base.
- ilist_iterator<T> (size-of: two pointers) operates on an
ilist_node<T>*, and downcasts to T* on dereference.
- ilist_sentinel<T> (size-of: two pointers) is a wrapper around
ilist_node<T> that has some extra API for list management.
- ilist<T> (size-of: two pointers) has an ilist_sentinel<T>, whose
address is returned for end().
The new memory layout matches ilist_half_embedded_sentinel_traits<T>
exactly. The Head pointer that previously lived in ilist<T> is
effectively glued to the ilist_half_node<T> that lived in
ilist_half_embedded_sentinel_traits<T>, becoming the Next and Prev in
the ilist_sentinel_node<T>, respectively. sizeof(ilist<T>) is now the
size of two pointers, and there is never any additional storage for a
sentinel.
This is a much simpler design for a doubly-linked list, removing most of
the corner cases of list manipulation (add, remove, etc.). In follow-up
commits, I intend to move as many algorithms as possible into a
non-templated base class (ilist_base) to reduce code size.
Moreover, this fixes the UB in ilist_iterator/getNext/getPrev
operations. Previously, ilist_iterator<T> operated on a T*, even when
the sentinel was not of type T (i.e., ilist_embedded_sentinel_traits and
ilist_half_embedded_sentinel_traits). This added UB to all operations
involving end(). Now, ilist_iterator<T> operates on an ilist_node<T>*,
and only downcasts when the full type is guaranteed to be T*.
What did we lose? There used to be a crash (in some configurations) on
++end(). Curiously (via UB), ++end() would return begin() for users of
ilist_half_embedded_sentinel_traits<T>, but otherwise ++end() would
cause a nice dependable nullptr dereference, crashing instead of a
possible infinite loop. Options:
1. Lose that behaviour.
2. Keep it, by stealing a bit from Prev in asserts builds.
3. Crash on dereference instead, using the same technique.
Hans convinced me (because of the number of problems this and r278532
exposed on Windows) that we really need some assertion here, at least in
the short term. I've opted for #3 since I think it catches more bugs.
I added only a couple of unit tests to root out specific bugs I hit
during bring-up, but otherwise this is tested implicitly via the
extensive usage throughout LLVM.
Planned follow-ups:
- Remove ilist_*sentinel_traits<T>. Here I've just gutted them to
prevent build failures in sub-projects. Once I stop referring to them
in sub-projects, I'll come back and delete them.
- Add ilist_base and move algorithms there.
- Check and fix move construction and assignment.
Eventually, there are other interesting directions:
- Rewrite reverse iterators, so that rbegin().getNodePtr()==&*rbegin().
This allows much simpler logic when erasing elements during a reverse
traversal.
- Remove ilist_traits::createNode, by deleting the remaining API that
creates nodes. Intrusive lists shouldn't be creating nodes
themselves.
- Remove ilist_traits::deleteNode, by (1) asserting that lists are empty
on destruction and (2) changing API that calls it to take a Deleter
functor (intrusive lists shouldn't be in the memory management
business).
- Reconfigure the remaining callback traits (addNodeToList, etc.) to be
higher-level, pulling out a simple_ilist<T> that is much easier to
read and understand.
- Allow tags (e.g., ilist_node<T,tag1> and ilist_node<T,tag2>) so that T
can be a member of multiple intrusive lists.
llvm-svn: 279314
2016-08-19 22:40:12 +02:00
|
|
|
// The ilist class is implemented as a circular list. The list itself contains
|
|
|
|
// a sentinel node, whose Next points at begin() and whose Prev points at
|
|
|
|
// rbegin(). The sentinel node itself serves as end() and rend().
|
2002-06-25 18:13:24 +02:00
|
|
|
//
|
|
|
|
//===----------------------------------------------------------------------===//
|
|
|
|
|
2008-05-29 20:17:53 +02:00
|
|
|
#ifndef LLVM_ADT_ILIST_H
|
|
|
|
#define LLVM_ADT_ILIST_H
|
2002-06-25 18:13:24 +02:00
|
|
|
|
ADT: Split out simple_ilist, a simple intrusive list
Split out a new, low-level intrusive list type with clear semantics.
Unlike iplist (and ilist), all operations on simple_ilist are intrusive,
and simple_ilist never takes ownership of its nodes. This enables an
intuitive API that has the right defaults for intrusive lists.
- insert() takes references (not pointers!) to nodes (in iplist/ilist,
passing a reference will cause the node to be copied).
- erase() takes only iterators (like std::list), and does not destroy
the nodes.
- remove() takes only references and has the same behaviour as erase().
- clear() does not destroy the nodes.
- The destructor does not destroy the nodes.
- New API {erase,remove,clear}AndDispose() take an extra Disposer
functor for callsites that want to call some disposal routine (e.g.,
std::default_delete).
This list is not currently configurable, and has no callbacks.
The initial motivation was to fix iplist<>::sort to work correctly (even
with callbacks in ilist_traits<>). iplist<> uses simple_ilist<>::sort
directly. The new test in unittests/IR/ModuleTest.cpp crashes without
this commit.
Fixing sort() via a low-level layer provided a good opportunity to:
- Unit test the low-level functionality thoroughly.
- Modernize the API, largely inspired by other intrusive list
implementations.
Here's a sketch of a longer-term plan:
- Create BumpPtrList<>, a non-intrusive list implemented using
simple_ilist<>, and use it for the Token list in
lib/Support/YAMLParser.cpp. This will factor out the only real use of
createNode().
- Evolve the iplist<> and ilist<> APIs in the direction of
simple_ilist<>, making allocation/deallocation explicit at call sites
(similar to simple_ilist<>::eraseAndDispose()).
- Factor out remaining calls to createNode() and deleteNode() and remove
the customization from ilist_traits<>.
- Transition uses of iplist<>/ilist<> that don't need callbacks over to
simple_ilist<>.
llvm-svn: 280107
2016-08-30 18:23:55 +02:00
|
|
|
#include "llvm/ADT/simple_ilist.h"
|
2012-09-17 08:31:17 +02:00
|
|
|
#include "llvm/Support/Compiler.h"
|
2003-10-15 23:55:37 +02:00
|
|
|
#include <cassert>
|
2010-06-10 12:13:58 +02:00
|
|
|
#include <cstddef>
|
2009-08-27 08:41:46 +02:00
|
|
|
#include <iterator>
|
ADT: Remove all ilist_iterator => pointer casts, NFC
Remove all ilist_iterator to pointer casts. There were two reasons for
casts:
- Checking for an uninitialized (i.e., null) iterator. I added
MachineInstrBundleIterator::isValid() to check for that case.
- Comparing an iterator against the underlying pointer value while
avoiding converting the pointer value to an iterator. This is
occasionally necessary in MachineInstrBundleIterator, since there is
an assertion in the constructors that the underlying MachineInstr is
not bundled (but we don't care about that if we're just checking for
pointer equality).
To support the latter case, I rewrote the == and != operators for
ilist_iterator and MachineInstrBundleIterator.
- The implicit constructors now use enable_if to exclude
const-iterator => non-const-iterator conversions from overload
resolution (previously it was a compiler error on instantiation, now
it's SFINAE).
- The == and != operators are now global (friends), and are not
templated.
- MachineInstrBundleIterator has overloads to compare against both
const_pointer and const_reference. This avoids the implicit
conversions to MachineInstrBundleIterator that assert, instead just
checking the address (and I added unit tests to confirm this).
Notably, the only remaining uses of ilist_iterator::getNodePtrUnchecked
are in ilist.h, and no code outside of ilist*.h directly relies on this
UB end-iterator-to-pointer conversion anymore. It's still needed for
ilist_*sentinel_traits, but I'll clean that up soon.
llvm-svn: 278478
2016-08-12 07:05:36 +02:00
|
|
|
#include <type_traits>
|
2002-06-25 18:13:24 +02:00
|
|
|
|
2003-11-11 23:41:34 +01:00
|
|
|
namespace llvm {
|
|
|
|
|
2016-08-30 20:40:47 +02:00
|
|
|
/// Use new/delete by default for iplist and ilist.
|
2016-08-30 19:01:05 +02:00
|
|
|
///
|
2016-08-30 20:40:47 +02:00
|
|
|
/// Specialize this to get different behaviour for allocation-related API. (If
|
|
|
|
/// you really want new/delete, consider just using std::list.)
|
|
|
|
///
|
|
|
|
/// \see ilist_noalloc_traits
|
|
|
|
template <typename NodeTy> struct ilist_alloc_traits {
|
ADT: Add AllocatorList, and use it for yaml::Token
- Add AllocatorList, a non-intrusive list that owns an LLVM-style
allocator and provides a std::list-like interface (trivially built on
top of simple_ilist),
- add a typedef (and unit tests) for BumpPtrList, and
- use BumpPtrList for the list of llvm::yaml::Token (i.e., TokenQueueT).
TokenQueueT has no need for the complexity of an intrusive list. The
only reason to inherit from ilist was to customize the allocator.
TokenQueueT was the only example in-tree of using ilist<> in a truly
non-intrusive way.
Moreover, this removes the final use of the non-intrusive
ilist_traits<>::createNode (after r280573, r281177, and r281181). I
have a WIP patch that removes this customization point (and the API that
relies on it) that I plan to commit soon.
Note: AllocatorList owns the allocator, which limits the viable API
(e.g., splicing must be on the same list). For now I've left out
any problematic API. It wouldn't be hard to split AllocatorList into
two layers: an Impl class that calls DerivedT::getAlloc (via CRTP), and
derived classes that handle Allocator ownership/reference/etc semantics;
and then implement splice with appropriate assertions; but TBH we should
probably just customize the std::list allocators at that point.
llvm-svn: 281182
2016-09-12 00:40:40 +02:00
|
|
|
/// Clone a node.
|
|
|
|
///
|
|
|
|
/// TODO: Remove this and API that relies on it (it's dead code).
|
2016-08-30 19:01:05 +02:00
|
|
|
static NodeTy *createNode(const NodeTy &V) { return new NodeTy(V); }
|
|
|
|
static void deleteNode(NodeTy *V) { delete V; }
|
2016-08-30 20:40:47 +02:00
|
|
|
};
|
2016-08-30 19:01:05 +02:00
|
|
|
|
2016-08-30 20:40:47 +02:00
|
|
|
/// Custom traits to disable node creation and do nothing on deletion.
|
|
|
|
///
|
|
|
|
/// Specialize ilist_alloc_traits to inherit from this to disable the
|
|
|
|
/// non-intrusive parts of iplist and/or ilist. It has no createNode function,
|
|
|
|
/// and deleteNode does nothing.
|
|
|
|
///
|
|
|
|
/// \code
|
|
|
|
/// template <>
|
|
|
|
/// struct ilist_alloc_traits<MyType> : ilist_noalloc_traits<MyType> {};
|
|
|
|
/// \endcode
|
|
|
|
template <typename NodeTy> struct ilist_noalloc_traits {
|
|
|
|
static void deleteNode(NodeTy *V) {}
|
|
|
|
};
|
|
|
|
|
|
|
|
/// Callbacks do nothing by default in iplist and ilist.
|
|
|
|
///
|
|
|
|
/// Specialize this for to use callbacks for when nodes change their list
|
|
|
|
/// membership.
|
|
|
|
template <typename NodeTy> struct ilist_callback_traits {
|
2016-08-30 19:01:05 +02:00
|
|
|
void addNodeToList(NodeTy *) {}
|
|
|
|
void removeNodeFromList(NodeTy *) {}
|
2016-08-30 20:00:45 +02:00
|
|
|
|
|
|
|
/// Callback before transferring nodes to this list.
|
|
|
|
///
|
|
|
|
/// \pre \c this!=&OldList
|
2016-08-30 20:40:47 +02:00
|
|
|
template <class Iterator>
|
|
|
|
void transferNodesFromList(ilist_callback_traits &OldList, Iterator /*first*/,
|
|
|
|
Iterator /*last*/) {
|
2016-08-30 20:00:45 +02:00
|
|
|
(void)OldList;
|
|
|
|
}
|
2016-08-30 19:01:05 +02:00
|
|
|
};
|
2016-08-30 20:40:47 +02:00
|
|
|
|
|
|
|
/// A fragment for template traits for intrusive list that provides default
|
|
|
|
/// node related operations.
|
|
|
|
///
|
|
|
|
/// TODO: Remove this layer of indirection. It's not necessary.
|
|
|
|
template <typename NodeTy>
|
|
|
|
struct ilist_node_traits : ilist_alloc_traits<NodeTy>,
|
|
|
|
ilist_callback_traits<NodeTy> {};
|
2016-08-30 19:01:05 +02:00
|
|
|
|
|
|
|
/// Default template traits for intrusive list.
|
|
|
|
///
|
|
|
|
/// By inheriting from this, you can easily use default implementations for all
|
|
|
|
/// common operations.
|
|
|
|
///
|
|
|
|
/// TODO: Remove this customization point. Specializing ilist_traits is
|
|
|
|
/// already fully general.
|
|
|
|
template <typename NodeTy>
|
|
|
|
struct ilist_default_traits : public ilist_node_traits<NodeTy> {};
|
|
|
|
|
|
|
|
/// Template traits for intrusive list.
|
|
|
|
///
|
|
|
|
/// Customize callbacks and allocation semantics.
|
|
|
|
template <typename NodeTy>
|
|
|
|
struct ilist_traits : public ilist_default_traits<NodeTy> {};
|
|
|
|
|
|
|
|
/// Const traits should never be instantiated.
|
|
|
|
template <typename Ty> struct ilist_traits<const Ty> {};
|
2016-08-12 19:32:34 +02:00
|
|
|
|
|
|
|
namespace ilist_detail {
|
|
|
|
|
|
|
|
template <class T> T &make();
|
|
|
|
|
|
|
|
/// Type trait to check for a traits class that has a getNext member (as a
|
|
|
|
/// canary for any of the ilist_nextprev_traits API).
|
2016-08-18 13:17:47 +02:00
|
|
|
template <class TraitsT, class NodeT> struct HasGetNext {
|
2016-08-12 19:32:34 +02:00
|
|
|
typedef char Yes[1];
|
|
|
|
typedef char No[2];
|
|
|
|
template <size_t N> struct SFINAE {};
|
|
|
|
|
2016-08-12 19:54:54 +02:00
|
|
|
template <class U>
|
2016-08-19 22:17:23 +02:00
|
|
|
static Yes &test(U *I, decltype(I->getNext(&make<NodeT>())) * = 0);
|
|
|
|
template <class> static No &test(...);
|
2016-08-12 19:32:34 +02:00
|
|
|
|
2016-08-19 22:17:23 +02:00
|
|
|
public:
|
|
|
|
static const bool value = sizeof(test<TraitsT>(nullptr)) == sizeof(Yes);
|
|
|
|
};
|
|
|
|
|
Reapply "ADT: Remove UB in ilist (and use a circular linked list)"
This reverts commit r279053, reapplying r278974 after fixing PR29035
with r279104.
Note that r279312 has been committed in the meantime, and this has been
rebased on top of that. Otherwise it's identical to r278974.
Note for maintainers of out-of-tree code (that I missed in the original
message): if the new isKnownSentinel() assertion is firing from
ilist_iterator<>::operator*(), this patch has identified a bug in your
code. There are a few common patterns:
- Some IR-related APIs htake an IRUnit* that might be nullptr, and pass
in an incremented iterator as an insertion point. Some old code was
using "&*++I", which in the case of end() only worked by fluke. If
the IRUnit in question inherits from ilist_node_with_parent<>, you can
use "I->getNextNode()". Otherwise, use "List.getNextNode(*I)".
- In most other cases, crashes on &*I just need to check for I==end()
before dereferencing.
- There's also occasional code that sends iterators into a function, and
then starts calling I->getOperand() (or other API). Either check for
end() before the entering the function, or early exit.
Note for if the static_assert with HasObsoleteCustomization is firing
for you:
- r278513 has examples of how to stop using custom sentinel traits.
- r278532 removed ilist_nextprev_traits since no one was using it. See
lld's r278469 for the only migration I needed to do.
Original commit message follows.
----
This removes the undefined behaviour (UB) in ilist/ilist_node/etc.,
mainly by removing (gutting) the ilist_sentinel_traits customization
point and canonicalizing on a single, efficient memory layout. This
fixes PR26753.
The new ilist is a doubly-linked circular list.
- ilist_node_base has two ilist_node_base*: Next and Prev. Size-of: two
pointers.
- ilist_node<T> (size-of: two pointers) is a type-safe wrapper around
ilist_node_base.
- ilist_iterator<T> (size-of: two pointers) operates on an
ilist_node<T>*, and downcasts to T* on dereference.
- ilist_sentinel<T> (size-of: two pointers) is a wrapper around
ilist_node<T> that has some extra API for list management.
- ilist<T> (size-of: two pointers) has an ilist_sentinel<T>, whose
address is returned for end().
The new memory layout matches ilist_half_embedded_sentinel_traits<T>
exactly. The Head pointer that previously lived in ilist<T> is
effectively glued to the ilist_half_node<T> that lived in
ilist_half_embedded_sentinel_traits<T>, becoming the Next and Prev in
the ilist_sentinel_node<T>, respectively. sizeof(ilist<T>) is now the
size of two pointers, and there is never any additional storage for a
sentinel.
This is a much simpler design for a doubly-linked list, removing most of
the corner cases of list manipulation (add, remove, etc.). In follow-up
commits, I intend to move as many algorithms as possible into a
non-templated base class (ilist_base) to reduce code size.
Moreover, this fixes the UB in ilist_iterator/getNext/getPrev
operations. Previously, ilist_iterator<T> operated on a T*, even when
the sentinel was not of type T (i.e., ilist_embedded_sentinel_traits and
ilist_half_embedded_sentinel_traits). This added UB to all operations
involving end(). Now, ilist_iterator<T> operates on an ilist_node<T>*,
and only downcasts when the full type is guaranteed to be T*.
What did we lose? There used to be a crash (in some configurations) on
++end(). Curiously (via UB), ++end() would return begin() for users of
ilist_half_embedded_sentinel_traits<T>, but otherwise ++end() would
cause a nice dependable nullptr dereference, crashing instead of a
possible infinite loop. Options:
1. Lose that behaviour.
2. Keep it, by stealing a bit from Prev in asserts builds.
3. Crash on dereference instead, using the same technique.
Hans convinced me (because of the number of problems this and r278532
exposed on Windows) that we really need some assertion here, at least in
the short term. I've opted for #3 since I think it catches more bugs.
I added only a couple of unit tests to root out specific bugs I hit
during bring-up, but otherwise this is tested implicitly via the
extensive usage throughout LLVM.
Planned follow-ups:
- Remove ilist_*sentinel_traits<T>. Here I've just gutted them to
prevent build failures in sub-projects. Once I stop referring to them
in sub-projects, I'll come back and delete them.
- Add ilist_base and move algorithms there.
- Check and fix move construction and assignment.
Eventually, there are other interesting directions:
- Rewrite reverse iterators, so that rbegin().getNodePtr()==&*rbegin().
This allows much simpler logic when erasing elements during a reverse
traversal.
- Remove ilist_traits::createNode, by deleting the remaining API that
creates nodes. Intrusive lists shouldn't be creating nodes
themselves.
- Remove ilist_traits::deleteNode, by (1) asserting that lists are empty
on destruction and (2) changing API that calls it to take a Deleter
functor (intrusive lists shouldn't be in the memory management
business).
- Reconfigure the remaining callback traits (addNodeToList, etc.) to be
higher-level, pulling out a simple_ilist<T> that is much easier to
read and understand.
- Allow tags (e.g., ilist_node<T,tag1> and ilist_node<T,tag2>) so that T
can be a member of multiple intrusive lists.
llvm-svn: 279314
2016-08-19 22:40:12 +02:00
|
|
|
/// Type trait to check for a traits class that has a createSentinel member (as
|
|
|
|
/// a canary for any of the ilist_sentinel_traits API).
|
|
|
|
template <class TraitsT> struct HasCreateSentinel {
|
|
|
|
typedef char Yes[1];
|
|
|
|
typedef char No[2];
|
2015-10-07 22:05:10 +02:00
|
|
|
|
Reapply "ADT: Remove UB in ilist (and use a circular linked list)"
This reverts commit r279053, reapplying r278974 after fixing PR29035
with r279104.
Note that r279312 has been committed in the meantime, and this has been
rebased on top of that. Otherwise it's identical to r278974.
Note for maintainers of out-of-tree code (that I missed in the original
message): if the new isKnownSentinel() assertion is firing from
ilist_iterator<>::operator*(), this patch has identified a bug in your
code. There are a few common patterns:
- Some IR-related APIs htake an IRUnit* that might be nullptr, and pass
in an incremented iterator as an insertion point. Some old code was
using "&*++I", which in the case of end() only worked by fluke. If
the IRUnit in question inherits from ilist_node_with_parent<>, you can
use "I->getNextNode()". Otherwise, use "List.getNextNode(*I)".
- In most other cases, crashes on &*I just need to check for I==end()
before dereferencing.
- There's also occasional code that sends iterators into a function, and
then starts calling I->getOperand() (or other API). Either check for
end() before the entering the function, or early exit.
Note for if the static_assert with HasObsoleteCustomization is firing
for you:
- r278513 has examples of how to stop using custom sentinel traits.
- r278532 removed ilist_nextprev_traits since no one was using it. See
lld's r278469 for the only migration I needed to do.
Original commit message follows.
----
This removes the undefined behaviour (UB) in ilist/ilist_node/etc.,
mainly by removing (gutting) the ilist_sentinel_traits customization
point and canonicalizing on a single, efficient memory layout. This
fixes PR26753.
The new ilist is a doubly-linked circular list.
- ilist_node_base has two ilist_node_base*: Next and Prev. Size-of: two
pointers.
- ilist_node<T> (size-of: two pointers) is a type-safe wrapper around
ilist_node_base.
- ilist_iterator<T> (size-of: two pointers) operates on an
ilist_node<T>*, and downcasts to T* on dereference.
- ilist_sentinel<T> (size-of: two pointers) is a wrapper around
ilist_node<T> that has some extra API for list management.
- ilist<T> (size-of: two pointers) has an ilist_sentinel<T>, whose
address is returned for end().
The new memory layout matches ilist_half_embedded_sentinel_traits<T>
exactly. The Head pointer that previously lived in ilist<T> is
effectively glued to the ilist_half_node<T> that lived in
ilist_half_embedded_sentinel_traits<T>, becoming the Next and Prev in
the ilist_sentinel_node<T>, respectively. sizeof(ilist<T>) is now the
size of two pointers, and there is never any additional storage for a
sentinel.
This is a much simpler design for a doubly-linked list, removing most of
the corner cases of list manipulation (add, remove, etc.). In follow-up
commits, I intend to move as many algorithms as possible into a
non-templated base class (ilist_base) to reduce code size.
Moreover, this fixes the UB in ilist_iterator/getNext/getPrev
operations. Previously, ilist_iterator<T> operated on a T*, even when
the sentinel was not of type T (i.e., ilist_embedded_sentinel_traits and
ilist_half_embedded_sentinel_traits). This added UB to all operations
involving end(). Now, ilist_iterator<T> operates on an ilist_node<T>*,
and only downcasts when the full type is guaranteed to be T*.
What did we lose? There used to be a crash (in some configurations) on
++end(). Curiously (via UB), ++end() would return begin() for users of
ilist_half_embedded_sentinel_traits<T>, but otherwise ++end() would
cause a nice dependable nullptr dereference, crashing instead of a
possible infinite loop. Options:
1. Lose that behaviour.
2. Keep it, by stealing a bit from Prev in asserts builds.
3. Crash on dereference instead, using the same technique.
Hans convinced me (because of the number of problems this and r278532
exposed on Windows) that we really need some assertion here, at least in
the short term. I've opted for #3 since I think it catches more bugs.
I added only a couple of unit tests to root out specific bugs I hit
during bring-up, but otherwise this is tested implicitly via the
extensive usage throughout LLVM.
Planned follow-ups:
- Remove ilist_*sentinel_traits<T>. Here I've just gutted them to
prevent build failures in sub-projects. Once I stop referring to them
in sub-projects, I'll come back and delete them.
- Add ilist_base and move algorithms there.
- Check and fix move construction and assignment.
Eventually, there are other interesting directions:
- Rewrite reverse iterators, so that rbegin().getNodePtr()==&*rbegin().
This allows much simpler logic when erasing elements during a reverse
traversal.
- Remove ilist_traits::createNode, by deleting the remaining API that
creates nodes. Intrusive lists shouldn't be creating nodes
themselves.
- Remove ilist_traits::deleteNode, by (1) asserting that lists are empty
on destruction and (2) changing API that calls it to take a Deleter
functor (intrusive lists shouldn't be in the memory management
business).
- Reconfigure the remaining callback traits (addNodeToList, etc.) to be
higher-level, pulling out a simple_ilist<T> that is much easier to
read and understand.
- Allow tags (e.g., ilist_node<T,tag1> and ilist_node<T,tag2>) so that T
can be a member of multiple intrusive lists.
llvm-svn: 279314
2016-08-19 22:40:12 +02:00
|
|
|
template <class U>
|
|
|
|
static Yes &test(U *I, decltype(I->createSentinel()) * = 0);
|
|
|
|
template <class> static No &test(...);
|
2016-08-18 13:17:53 +02:00
|
|
|
|
Reapply "ADT: Remove UB in ilist (and use a circular linked list)"
This reverts commit r279053, reapplying r278974 after fixing PR29035
with r279104.
Note that r279312 has been committed in the meantime, and this has been
rebased on top of that. Otherwise it's identical to r278974.
Note for maintainers of out-of-tree code (that I missed in the original
message): if the new isKnownSentinel() assertion is firing from
ilist_iterator<>::operator*(), this patch has identified a bug in your
code. There are a few common patterns:
- Some IR-related APIs htake an IRUnit* that might be nullptr, and pass
in an incremented iterator as an insertion point. Some old code was
using "&*++I", which in the case of end() only worked by fluke. If
the IRUnit in question inherits from ilist_node_with_parent<>, you can
use "I->getNextNode()". Otherwise, use "List.getNextNode(*I)".
- In most other cases, crashes on &*I just need to check for I==end()
before dereferencing.
- There's also occasional code that sends iterators into a function, and
then starts calling I->getOperand() (or other API). Either check for
end() before the entering the function, or early exit.
Note for if the static_assert with HasObsoleteCustomization is firing
for you:
- r278513 has examples of how to stop using custom sentinel traits.
- r278532 removed ilist_nextprev_traits since no one was using it. See
lld's r278469 for the only migration I needed to do.
Original commit message follows.
----
This removes the undefined behaviour (UB) in ilist/ilist_node/etc.,
mainly by removing (gutting) the ilist_sentinel_traits customization
point and canonicalizing on a single, efficient memory layout. This
fixes PR26753.
The new ilist is a doubly-linked circular list.
- ilist_node_base has two ilist_node_base*: Next and Prev. Size-of: two
pointers.
- ilist_node<T> (size-of: two pointers) is a type-safe wrapper around
ilist_node_base.
- ilist_iterator<T> (size-of: two pointers) operates on an
ilist_node<T>*, and downcasts to T* on dereference.
- ilist_sentinel<T> (size-of: two pointers) is a wrapper around
ilist_node<T> that has some extra API for list management.
- ilist<T> (size-of: two pointers) has an ilist_sentinel<T>, whose
address is returned for end().
The new memory layout matches ilist_half_embedded_sentinel_traits<T>
exactly. The Head pointer that previously lived in ilist<T> is
effectively glued to the ilist_half_node<T> that lived in
ilist_half_embedded_sentinel_traits<T>, becoming the Next and Prev in
the ilist_sentinel_node<T>, respectively. sizeof(ilist<T>) is now the
size of two pointers, and there is never any additional storage for a
sentinel.
This is a much simpler design for a doubly-linked list, removing most of
the corner cases of list manipulation (add, remove, etc.). In follow-up
commits, I intend to move as many algorithms as possible into a
non-templated base class (ilist_base) to reduce code size.
Moreover, this fixes the UB in ilist_iterator/getNext/getPrev
operations. Previously, ilist_iterator<T> operated on a T*, even when
the sentinel was not of type T (i.e., ilist_embedded_sentinel_traits and
ilist_half_embedded_sentinel_traits). This added UB to all operations
involving end(). Now, ilist_iterator<T> operates on an ilist_node<T>*,
and only downcasts when the full type is guaranteed to be T*.
What did we lose? There used to be a crash (in some configurations) on
++end(). Curiously (via UB), ++end() would return begin() for users of
ilist_half_embedded_sentinel_traits<T>, but otherwise ++end() would
cause a nice dependable nullptr dereference, crashing instead of a
possible infinite loop. Options:
1. Lose that behaviour.
2. Keep it, by stealing a bit from Prev in asserts builds.
3. Crash on dereference instead, using the same technique.
Hans convinced me (because of the number of problems this and r278532
exposed on Windows) that we really need some assertion here, at least in
the short term. I've opted for #3 since I think it catches more bugs.
I added only a couple of unit tests to root out specific bugs I hit
during bring-up, but otherwise this is tested implicitly via the
extensive usage throughout LLVM.
Planned follow-ups:
- Remove ilist_*sentinel_traits<T>. Here I've just gutted them to
prevent build failures in sub-projects. Once I stop referring to them
in sub-projects, I'll come back and delete them.
- Add ilist_base and move algorithms there.
- Check and fix move construction and assignment.
Eventually, there are other interesting directions:
- Rewrite reverse iterators, so that rbegin().getNodePtr()==&*rbegin().
This allows much simpler logic when erasing elements during a reverse
traversal.
- Remove ilist_traits::createNode, by deleting the remaining API that
creates nodes. Intrusive lists shouldn't be creating nodes
themselves.
- Remove ilist_traits::deleteNode, by (1) asserting that lists are empty
on destruction and (2) changing API that calls it to take a Deleter
functor (intrusive lists shouldn't be in the memory management
business).
- Reconfigure the remaining callback traits (addNodeToList, etc.) to be
higher-level, pulling out a simple_ilist<T> that is much easier to
read and understand.
- Allow tags (e.g., ilist_node<T,tag1> and ilist_node<T,tag2>) so that T
can be a member of multiple intrusive lists.
llvm-svn: 279314
2016-08-19 22:40:12 +02:00
|
|
|
public:
|
|
|
|
static const bool value = sizeof(test<TraitsT>(nullptr)) == sizeof(Yes);
|
2015-10-07 22:05:10 +02:00
|
|
|
};
|
|
|
|
|
Reapply "ADT: Remove UB in ilist (and use a circular linked list)"
This reverts commit r279053, reapplying r278974 after fixing PR29035
with r279104.
Note that r279312 has been committed in the meantime, and this has been
rebased on top of that. Otherwise it's identical to r278974.
Note for maintainers of out-of-tree code (that I missed in the original
message): if the new isKnownSentinel() assertion is firing from
ilist_iterator<>::operator*(), this patch has identified a bug in your
code. There are a few common patterns:
- Some IR-related APIs htake an IRUnit* that might be nullptr, and pass
in an incremented iterator as an insertion point. Some old code was
using "&*++I", which in the case of end() only worked by fluke. If
the IRUnit in question inherits from ilist_node_with_parent<>, you can
use "I->getNextNode()". Otherwise, use "List.getNextNode(*I)".
- In most other cases, crashes on &*I just need to check for I==end()
before dereferencing.
- There's also occasional code that sends iterators into a function, and
then starts calling I->getOperand() (or other API). Either check for
end() before the entering the function, or early exit.
Note for if the static_assert with HasObsoleteCustomization is firing
for you:
- r278513 has examples of how to stop using custom sentinel traits.
- r278532 removed ilist_nextprev_traits since no one was using it. See
lld's r278469 for the only migration I needed to do.
Original commit message follows.
----
This removes the undefined behaviour (UB) in ilist/ilist_node/etc.,
mainly by removing (gutting) the ilist_sentinel_traits customization
point and canonicalizing on a single, efficient memory layout. This
fixes PR26753.
The new ilist is a doubly-linked circular list.
- ilist_node_base has two ilist_node_base*: Next and Prev. Size-of: two
pointers.
- ilist_node<T> (size-of: two pointers) is a type-safe wrapper around
ilist_node_base.
- ilist_iterator<T> (size-of: two pointers) operates on an
ilist_node<T>*, and downcasts to T* on dereference.
- ilist_sentinel<T> (size-of: two pointers) is a wrapper around
ilist_node<T> that has some extra API for list management.
- ilist<T> (size-of: two pointers) has an ilist_sentinel<T>, whose
address is returned for end().
The new memory layout matches ilist_half_embedded_sentinel_traits<T>
exactly. The Head pointer that previously lived in ilist<T> is
effectively glued to the ilist_half_node<T> that lived in
ilist_half_embedded_sentinel_traits<T>, becoming the Next and Prev in
the ilist_sentinel_node<T>, respectively. sizeof(ilist<T>) is now the
size of two pointers, and there is never any additional storage for a
sentinel.
This is a much simpler design for a doubly-linked list, removing most of
the corner cases of list manipulation (add, remove, etc.). In follow-up
commits, I intend to move as many algorithms as possible into a
non-templated base class (ilist_base) to reduce code size.
Moreover, this fixes the UB in ilist_iterator/getNext/getPrev
operations. Previously, ilist_iterator<T> operated on a T*, even when
the sentinel was not of type T (i.e., ilist_embedded_sentinel_traits and
ilist_half_embedded_sentinel_traits). This added UB to all operations
involving end(). Now, ilist_iterator<T> operates on an ilist_node<T>*,
and only downcasts when the full type is guaranteed to be T*.
What did we lose? There used to be a crash (in some configurations) on
++end(). Curiously (via UB), ++end() would return begin() for users of
ilist_half_embedded_sentinel_traits<T>, but otherwise ++end() would
cause a nice dependable nullptr dereference, crashing instead of a
possible infinite loop. Options:
1. Lose that behaviour.
2. Keep it, by stealing a bit from Prev in asserts builds.
3. Crash on dereference instead, using the same technique.
Hans convinced me (because of the number of problems this and r278532
exposed on Windows) that we really need some assertion here, at least in
the short term. I've opted for #3 since I think it catches more bugs.
I added only a couple of unit tests to root out specific bugs I hit
during bring-up, but otherwise this is tested implicitly via the
extensive usage throughout LLVM.
Planned follow-ups:
- Remove ilist_*sentinel_traits<T>. Here I've just gutted them to
prevent build failures in sub-projects. Once I stop referring to them
in sub-projects, I'll come back and delete them.
- Add ilist_base and move algorithms there.
- Check and fix move construction and assignment.
Eventually, there are other interesting directions:
- Rewrite reverse iterators, so that rbegin().getNodePtr()==&*rbegin().
This allows much simpler logic when erasing elements during a reverse
traversal.
- Remove ilist_traits::createNode, by deleting the remaining API that
creates nodes. Intrusive lists shouldn't be creating nodes
themselves.
- Remove ilist_traits::deleteNode, by (1) asserting that lists are empty
on destruction and (2) changing API that calls it to take a Deleter
functor (intrusive lists shouldn't be in the memory management
business).
- Reconfigure the remaining callback traits (addNodeToList, etc.) to be
higher-level, pulling out a simple_ilist<T> that is much easier to
read and understand.
- Allow tags (e.g., ilist_node<T,tag1> and ilist_node<T,tag2>) so that T
can be a member of multiple intrusive lists.
llvm-svn: 279314
2016-08-19 22:40:12 +02:00
|
|
|
template <class TraitsT, class NodeT> struct HasObsoleteCustomization {
|
|
|
|
static const bool value =
|
|
|
|
HasGetNext<TraitsT, NodeT>::value || HasCreateSentinel<TraitsT>::value;
|
2016-08-18 13:17:53 +02:00
|
|
|
};
|
|
|
|
|
Reapply "ADT: Remove UB in ilist (and use a circular linked list)"
This reverts commit r279053, reapplying r278974 after fixing PR29035
with r279104.
Note that r279312 has been committed in the meantime, and this has been
rebased on top of that. Otherwise it's identical to r278974.
Note for maintainers of out-of-tree code (that I missed in the original
message): if the new isKnownSentinel() assertion is firing from
ilist_iterator<>::operator*(), this patch has identified a bug in your
code. There are a few common patterns:
- Some IR-related APIs htake an IRUnit* that might be nullptr, and pass
in an incremented iterator as an insertion point. Some old code was
using "&*++I", which in the case of end() only worked by fluke. If
the IRUnit in question inherits from ilist_node_with_parent<>, you can
use "I->getNextNode()". Otherwise, use "List.getNextNode(*I)".
- In most other cases, crashes on &*I just need to check for I==end()
before dereferencing.
- There's also occasional code that sends iterators into a function, and
then starts calling I->getOperand() (or other API). Either check for
end() before the entering the function, or early exit.
Note for if the static_assert with HasObsoleteCustomization is firing
for you:
- r278513 has examples of how to stop using custom sentinel traits.
- r278532 removed ilist_nextprev_traits since no one was using it. See
lld's r278469 for the only migration I needed to do.
Original commit message follows.
----
This removes the undefined behaviour (UB) in ilist/ilist_node/etc.,
mainly by removing (gutting) the ilist_sentinel_traits customization
point and canonicalizing on a single, efficient memory layout. This
fixes PR26753.
The new ilist is a doubly-linked circular list.
- ilist_node_base has two ilist_node_base*: Next and Prev. Size-of: two
pointers.
- ilist_node<T> (size-of: two pointers) is a type-safe wrapper around
ilist_node_base.
- ilist_iterator<T> (size-of: two pointers) operates on an
ilist_node<T>*, and downcasts to T* on dereference.
- ilist_sentinel<T> (size-of: two pointers) is a wrapper around
ilist_node<T> that has some extra API for list management.
- ilist<T> (size-of: two pointers) has an ilist_sentinel<T>, whose
address is returned for end().
The new memory layout matches ilist_half_embedded_sentinel_traits<T>
exactly. The Head pointer that previously lived in ilist<T> is
effectively glued to the ilist_half_node<T> that lived in
ilist_half_embedded_sentinel_traits<T>, becoming the Next and Prev in
the ilist_sentinel_node<T>, respectively. sizeof(ilist<T>) is now the
size of two pointers, and there is never any additional storage for a
sentinel.
This is a much simpler design for a doubly-linked list, removing most of
the corner cases of list manipulation (add, remove, etc.). In follow-up
commits, I intend to move as many algorithms as possible into a
non-templated base class (ilist_base) to reduce code size.
Moreover, this fixes the UB in ilist_iterator/getNext/getPrev
operations. Previously, ilist_iterator<T> operated on a T*, even when
the sentinel was not of type T (i.e., ilist_embedded_sentinel_traits and
ilist_half_embedded_sentinel_traits). This added UB to all operations
involving end(). Now, ilist_iterator<T> operates on an ilist_node<T>*,
and only downcasts when the full type is guaranteed to be T*.
What did we lose? There used to be a crash (in some configurations) on
++end(). Curiously (via UB), ++end() would return begin() for users of
ilist_half_embedded_sentinel_traits<T>, but otherwise ++end() would
cause a nice dependable nullptr dereference, crashing instead of a
possible infinite loop. Options:
1. Lose that behaviour.
2. Keep it, by stealing a bit from Prev in asserts builds.
3. Crash on dereference instead, using the same technique.
Hans convinced me (because of the number of problems this and r278532
exposed on Windows) that we really need some assertion here, at least in
the short term. I've opted for #3 since I think it catches more bugs.
I added only a couple of unit tests to root out specific bugs I hit
during bring-up, but otherwise this is tested implicitly via the
extensive usage throughout LLVM.
Planned follow-ups:
- Remove ilist_*sentinel_traits<T>. Here I've just gutted them to
prevent build failures in sub-projects. Once I stop referring to them
in sub-projects, I'll come back and delete them.
- Add ilist_base and move algorithms there.
- Check and fix move construction and assignment.
Eventually, there are other interesting directions:
- Rewrite reverse iterators, so that rbegin().getNodePtr()==&*rbegin().
This allows much simpler logic when erasing elements during a reverse
traversal.
- Remove ilist_traits::createNode, by deleting the remaining API that
creates nodes. Intrusive lists shouldn't be creating nodes
themselves.
- Remove ilist_traits::deleteNode, by (1) asserting that lists are empty
on destruction and (2) changing API that calls it to take a Deleter
functor (intrusive lists shouldn't be in the memory management
business).
- Reconfigure the remaining callback traits (addNodeToList, etc.) to be
higher-level, pulling out a simple_ilist<T> that is much easier to
read and understand.
- Allow tags (e.g., ilist_node<T,tag1> and ilist_node<T,tag2>) so that T
can be a member of multiple intrusive lists.
llvm-svn: 279314
2016-08-19 22:40:12 +02:00
|
|
|
} // end namespace ilist_detail
|
2016-08-12 17:00:55 +02:00
|
|
|
|
2002-06-25 18:13:24 +02:00
|
|
|
//===----------------------------------------------------------------------===//
|
|
|
|
//
|
2016-09-03 04:07:45 +02:00
|
|
|
/// A wrapper around an intrusive list with callbacks and non-intrusive
|
|
|
|
/// ownership.
|
|
|
|
///
|
|
|
|
/// This wraps a purely intrusive list (like simple_ilist) with a configurable
|
|
|
|
/// traits class. The traits can implement callbacks and customize the
|
|
|
|
/// ownership semantics.
|
|
|
|
///
|
|
|
|
/// This is a subset of ilist functionality that can safely be used on nodes of
|
Reapply "ADT: Remove UB in ilist (and use a circular linked list)"
This reverts commit r279053, reapplying r278974 after fixing PR29035
with r279104.
Note that r279312 has been committed in the meantime, and this has been
rebased on top of that. Otherwise it's identical to r278974.
Note for maintainers of out-of-tree code (that I missed in the original
message): if the new isKnownSentinel() assertion is firing from
ilist_iterator<>::operator*(), this patch has identified a bug in your
code. There are a few common patterns:
- Some IR-related APIs htake an IRUnit* that might be nullptr, and pass
in an incremented iterator as an insertion point. Some old code was
using "&*++I", which in the case of end() only worked by fluke. If
the IRUnit in question inherits from ilist_node_with_parent<>, you can
use "I->getNextNode()". Otherwise, use "List.getNextNode(*I)".
- In most other cases, crashes on &*I just need to check for I==end()
before dereferencing.
- There's also occasional code that sends iterators into a function, and
then starts calling I->getOperand() (or other API). Either check for
end() before the entering the function, or early exit.
Note for if the static_assert with HasObsoleteCustomization is firing
for you:
- r278513 has examples of how to stop using custom sentinel traits.
- r278532 removed ilist_nextprev_traits since no one was using it. See
lld's r278469 for the only migration I needed to do.
Original commit message follows.
----
This removes the undefined behaviour (UB) in ilist/ilist_node/etc.,
mainly by removing (gutting) the ilist_sentinel_traits customization
point and canonicalizing on a single, efficient memory layout. This
fixes PR26753.
The new ilist is a doubly-linked circular list.
- ilist_node_base has two ilist_node_base*: Next and Prev. Size-of: two
pointers.
- ilist_node<T> (size-of: two pointers) is a type-safe wrapper around
ilist_node_base.
- ilist_iterator<T> (size-of: two pointers) operates on an
ilist_node<T>*, and downcasts to T* on dereference.
- ilist_sentinel<T> (size-of: two pointers) is a wrapper around
ilist_node<T> that has some extra API for list management.
- ilist<T> (size-of: two pointers) has an ilist_sentinel<T>, whose
address is returned for end().
The new memory layout matches ilist_half_embedded_sentinel_traits<T>
exactly. The Head pointer that previously lived in ilist<T> is
effectively glued to the ilist_half_node<T> that lived in
ilist_half_embedded_sentinel_traits<T>, becoming the Next and Prev in
the ilist_sentinel_node<T>, respectively. sizeof(ilist<T>) is now the
size of two pointers, and there is never any additional storage for a
sentinel.
This is a much simpler design for a doubly-linked list, removing most of
the corner cases of list manipulation (add, remove, etc.). In follow-up
commits, I intend to move as many algorithms as possible into a
non-templated base class (ilist_base) to reduce code size.
Moreover, this fixes the UB in ilist_iterator/getNext/getPrev
operations. Previously, ilist_iterator<T> operated on a T*, even when
the sentinel was not of type T (i.e., ilist_embedded_sentinel_traits and
ilist_half_embedded_sentinel_traits). This added UB to all operations
involving end(). Now, ilist_iterator<T> operates on an ilist_node<T>*,
and only downcasts when the full type is guaranteed to be T*.
What did we lose? There used to be a crash (in some configurations) on
++end(). Curiously (via UB), ++end() would return begin() for users of
ilist_half_embedded_sentinel_traits<T>, but otherwise ++end() would
cause a nice dependable nullptr dereference, crashing instead of a
possible infinite loop. Options:
1. Lose that behaviour.
2. Keep it, by stealing a bit from Prev in asserts builds.
3. Crash on dereference instead, using the same technique.
Hans convinced me (because of the number of problems this and r278532
exposed on Windows) that we really need some assertion here, at least in
the short term. I've opted for #3 since I think it catches more bugs.
I added only a couple of unit tests to root out specific bugs I hit
during bring-up, but otherwise this is tested implicitly via the
extensive usage throughout LLVM.
Planned follow-ups:
- Remove ilist_*sentinel_traits<T>. Here I've just gutted them to
prevent build failures in sub-projects. Once I stop referring to them
in sub-projects, I'll come back and delete them.
- Add ilist_base and move algorithms there.
- Check and fix move construction and assignment.
Eventually, there are other interesting directions:
- Rewrite reverse iterators, so that rbegin().getNodePtr()==&*rbegin().
This allows much simpler logic when erasing elements during a reverse
traversal.
- Remove ilist_traits::createNode, by deleting the remaining API that
creates nodes. Intrusive lists shouldn't be creating nodes
themselves.
- Remove ilist_traits::deleteNode, by (1) asserting that lists are empty
on destruction and (2) changing API that calls it to take a Deleter
functor (intrusive lists shouldn't be in the memory management
business).
- Reconfigure the remaining callback traits (addNodeToList, etc.) to be
higher-level, pulling out a simple_ilist<T> that is much easier to
read and understand.
- Allow tags (e.g., ilist_node<T,tag1> and ilist_node<T,tag2>) so that T
can be a member of multiple intrusive lists.
llvm-svn: 279314
2016-08-19 22:40:12 +02:00
|
|
|
/// polymorphic types, i.e. a heterogeneous list with a common base class that
|
|
|
|
/// holds the next/prev pointers. The only state of the list itself is an
|
|
|
|
/// ilist_sentinel, which holds pointers to the first and last nodes in the
|
|
|
|
/// list.
|
2016-09-03 04:07:45 +02:00
|
|
|
template <class IntrusiveListT, class TraitsT>
|
|
|
|
class iplist_impl : public TraitsT, IntrusiveListT {
|
|
|
|
typedef IntrusiveListT base_list_type;
|
2008-07-07 20:43:32 +02:00
|
|
|
|
2016-09-12 00:11:37 +02:00
|
|
|
protected:
|
|
|
|
typedef iplist_impl iplist_impl_type;
|
|
|
|
|
2002-06-25 18:13:24 +02:00
|
|
|
public:
|
ADT: Split out simple_ilist, a simple intrusive list
Split out a new, low-level intrusive list type with clear semantics.
Unlike iplist (and ilist), all operations on simple_ilist are intrusive,
and simple_ilist never takes ownership of its nodes. This enables an
intuitive API that has the right defaults for intrusive lists.
- insert() takes references (not pointers!) to nodes (in iplist/ilist,
passing a reference will cause the node to be copied).
- erase() takes only iterators (like std::list), and does not destroy
the nodes.
- remove() takes only references and has the same behaviour as erase().
- clear() does not destroy the nodes.
- The destructor does not destroy the nodes.
- New API {erase,remove,clear}AndDispose() take an extra Disposer
functor for callsites that want to call some disposal routine (e.g.,
std::default_delete).
This list is not currently configurable, and has no callbacks.
The initial motivation was to fix iplist<>::sort to work correctly (even
with callbacks in ilist_traits<>). iplist<> uses simple_ilist<>::sort
directly. The new test in unittests/IR/ModuleTest.cpp crashes without
this commit.
Fixing sort() via a low-level layer provided a good opportunity to:
- Unit test the low-level functionality thoroughly.
- Modernize the API, largely inspired by other intrusive list
implementations.
Here's a sketch of a longer-term plan:
- Create BumpPtrList<>, a non-intrusive list implemented using
simple_ilist<>, and use it for the Token list in
lib/Support/YAMLParser.cpp. This will factor out the only real use of
createNode().
- Evolve the iplist<> and ilist<> APIs in the direction of
simple_ilist<>, making allocation/deallocation explicit at call sites
(similar to simple_ilist<>::eraseAndDispose()).
- Factor out remaining calls to createNode() and deleteNode() and remove
the customization from ilist_traits<>.
- Transition uses of iplist<>/ilist<> that don't need callbacks over to
simple_ilist<>.
llvm-svn: 280107
2016-08-30 18:23:55 +02:00
|
|
|
typedef typename base_list_type::pointer pointer;
|
|
|
|
typedef typename base_list_type::const_pointer const_pointer;
|
|
|
|
typedef typename base_list_type::reference reference;
|
|
|
|
typedef typename base_list_type::const_reference const_reference;
|
|
|
|
typedef typename base_list_type::value_type value_type;
|
|
|
|
typedef typename base_list_type::size_type size_type;
|
|
|
|
typedef typename base_list_type::difference_type difference_type;
|
|
|
|
typedef typename base_list_type::iterator iterator;
|
|
|
|
typedef typename base_list_type::const_iterator const_iterator;
|
|
|
|
typedef typename base_list_type::reverse_iterator reverse_iterator;
|
|
|
|
typedef
|
|
|
|
typename base_list_type::const_reverse_iterator const_reverse_iterator;
|
2002-06-25 18:13:24 +02:00
|
|
|
|
2016-09-03 03:42:40 +02:00
|
|
|
private:
|
|
|
|
// 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).
|
2016-09-03 04:07:45 +02:00
|
|
|
static_assert(
|
|
|
|
!ilist_detail::HasObsoleteCustomization<TraitsT, value_type>::value,
|
|
|
|
"ilist customization points have changed!");
|
2016-09-03 03:42:40 +02:00
|
|
|
|
|
|
|
static bool op_less(const_reference L, const_reference R) { return L < R; }
|
|
|
|
static bool op_equal(const_reference L, const_reference R) { return L == R; }
|
|
|
|
|
|
|
|
// Copying intrusively linked nodes doesn't make sense.
|
2016-09-03 04:07:45 +02:00
|
|
|
iplist_impl(const iplist_impl &) = delete;
|
|
|
|
void operator=(const iplist_impl &) = delete;
|
2016-09-03 03:42:40 +02:00
|
|
|
|
|
|
|
public:
|
2016-09-03 04:07:45 +02:00
|
|
|
iplist_impl() = default;
|
2016-09-12 00:11:37 +02:00
|
|
|
iplist_impl(iplist_impl &&X)
|
|
|
|
: TraitsT(std::move(X)), IntrusiveListT(std::move(X)) {}
|
|
|
|
iplist_impl &operator=(iplist_impl &&X) {
|
|
|
|
*static_cast<TraitsT *>(this) = std::move(X);
|
|
|
|
*static_cast<IntrusiveListT *>(this) = std::move(X);
|
|
|
|
return *this;
|
|
|
|
}
|
2016-09-03 04:07:45 +02:00
|
|
|
~iplist_impl() { clear(); }
|
2002-06-25 18:13:24 +02:00
|
|
|
|
2004-02-08 01:51:31 +01:00
|
|
|
// Miscellaneous inspection routines.
|
2002-06-25 18:13:24 +02:00
|
|
|
size_type max_size() const { return size_type(-1); }
|
|
|
|
|
ADT: Split out simple_ilist, a simple intrusive list
Split out a new, low-level intrusive list type with clear semantics.
Unlike iplist (and ilist), all operations on simple_ilist are intrusive,
and simple_ilist never takes ownership of its nodes. This enables an
intuitive API that has the right defaults for intrusive lists.
- insert() takes references (not pointers!) to nodes (in iplist/ilist,
passing a reference will cause the node to be copied).
- erase() takes only iterators (like std::list), and does not destroy
the nodes.
- remove() takes only references and has the same behaviour as erase().
- clear() does not destroy the nodes.
- The destructor does not destroy the nodes.
- New API {erase,remove,clear}AndDispose() take an extra Disposer
functor for callsites that want to call some disposal routine (e.g.,
std::default_delete).
This list is not currently configurable, and has no callbacks.
The initial motivation was to fix iplist<>::sort to work correctly (even
with callbacks in ilist_traits<>). iplist<> uses simple_ilist<>::sort
directly. The new test in unittests/IR/ModuleTest.cpp crashes without
this commit.
Fixing sort() via a low-level layer provided a good opportunity to:
- Unit test the low-level functionality thoroughly.
- Modernize the API, largely inspired by other intrusive list
implementations.
Here's a sketch of a longer-term plan:
- Create BumpPtrList<>, a non-intrusive list implemented using
simple_ilist<>, and use it for the Token list in
lib/Support/YAMLParser.cpp. This will factor out the only real use of
createNode().
- Evolve the iplist<> and ilist<> APIs in the direction of
simple_ilist<>, making allocation/deallocation explicit at call sites
(similar to simple_ilist<>::eraseAndDispose()).
- Factor out remaining calls to createNode() and deleteNode() and remove
the customization from ilist_traits<>.
- Transition uses of iplist<>/ilist<> that don't need callbacks over to
simple_ilist<>.
llvm-svn: 280107
2016-08-30 18:23:55 +02:00
|
|
|
using base_list_type::begin;
|
|
|
|
using base_list_type::end;
|
|
|
|
using base_list_type::rbegin;
|
|
|
|
using base_list_type::rend;
|
|
|
|
using base_list_type::empty;
|
|
|
|
using base_list_type::front;
|
|
|
|
using base_list_type::back;
|
2002-06-25 18:13:24 +02:00
|
|
|
|
2016-09-03 04:07:45 +02:00
|
|
|
void swap(iplist_impl &RHS) {
|
2009-01-05 18:59:02 +01:00
|
|
|
assert(0 && "Swap does not use list traits callback correctly yet!");
|
ADT: Split out simple_ilist, a simple intrusive list
Split out a new, low-level intrusive list type with clear semantics.
Unlike iplist (and ilist), all operations on simple_ilist are intrusive,
and simple_ilist never takes ownership of its nodes. This enables an
intuitive API that has the right defaults for intrusive lists.
- insert() takes references (not pointers!) to nodes (in iplist/ilist,
passing a reference will cause the node to be copied).
- erase() takes only iterators (like std::list), and does not destroy
the nodes.
- remove() takes only references and has the same behaviour as erase().
- clear() does not destroy the nodes.
- The destructor does not destroy the nodes.
- New API {erase,remove,clear}AndDispose() take an extra Disposer
functor for callsites that want to call some disposal routine (e.g.,
std::default_delete).
This list is not currently configurable, and has no callbacks.
The initial motivation was to fix iplist<>::sort to work correctly (even
with callbacks in ilist_traits<>). iplist<> uses simple_ilist<>::sort
directly. The new test in unittests/IR/ModuleTest.cpp crashes without
this commit.
Fixing sort() via a low-level layer provided a good opportunity to:
- Unit test the low-level functionality thoroughly.
- Modernize the API, largely inspired by other intrusive list
implementations.
Here's a sketch of a longer-term plan:
- Create BumpPtrList<>, a non-intrusive list implemented using
simple_ilist<>, and use it for the Token list in
lib/Support/YAMLParser.cpp. This will factor out the only real use of
createNode().
- Evolve the iplist<> and ilist<> APIs in the direction of
simple_ilist<>, making allocation/deallocation explicit at call sites
(similar to simple_ilist<>::eraseAndDispose()).
- Factor out remaining calls to createNode() and deleteNode() and remove
the customization from ilist_traits<>.
- Transition uses of iplist<>/ilist<> that don't need callbacks over to
simple_ilist<>.
llvm-svn: 280107
2016-08-30 18:23:55 +02:00
|
|
|
base_list_type::swap(RHS);
|
2002-06-25 18:13:24 +02:00
|
|
|
}
|
|
|
|
|
2016-09-03 03:42:40 +02:00
|
|
|
iterator insert(iterator where, pointer New) {
|
2016-08-30 20:00:45 +02:00
|
|
|
this->addNodeToList(New); // Notify traits that we added a node...
|
|
|
|
return base_list_type::insert(where, *New);
|
2002-06-25 18:13:24 +02:00
|
|
|
}
|
|
|
|
|
2016-09-03 03:42:40 +02:00
|
|
|
iterator insert(iterator where, const_reference New) {
|
|
|
|
return this->insert(where, new value_type(New));
|
2016-07-17 00:51:33 +02:00
|
|
|
}
|
|
|
|
|
2016-09-03 03:42:40 +02:00
|
|
|
iterator insertAfter(iterator where, pointer New) {
|
2009-02-20 23:54:36 +01:00
|
|
|
if (empty())
|
2009-01-13 08:43:51 +01:00
|
|
|
return insert(begin(), New);
|
|
|
|
else
|
|
|
|
return insert(++where, New);
|
|
|
|
}
|
|
|
|
|
2016-09-03 03:42:40 +02:00
|
|
|
pointer remove(iterator &IT) {
|
|
|
|
pointer Node = &*IT++;
|
2016-08-30 20:00:45 +02:00
|
|
|
this->removeNodeFromList(Node); // Notify traits that we removed a node...
|
|
|
|
base_list_type::remove(*Node);
|
2002-06-25 18:13:24 +02:00
|
|
|
return Node;
|
|
|
|
}
|
|
|
|
|
2016-09-03 03:42:40 +02:00
|
|
|
pointer remove(const iterator &IT) {
|
2002-06-25 18:13:24 +02:00
|
|
|
iterator MutIt = IT;
|
|
|
|
return remove(MutIt);
|
|
|
|
}
|
|
|
|
|
2016-09-03 03:42:40 +02:00
|
|
|
pointer remove(pointer IT) { return remove(iterator(IT)); }
|
|
|
|
pointer remove(reference IT) { return remove(iterator(IT)); }
|
2015-10-09 20:23:49 +02:00
|
|
|
|
2002-06-25 18:13:24 +02:00
|
|
|
// erase - remove a node from the controlled sequence... and delete it.
|
|
|
|
iterator erase(iterator where) {
|
2009-01-21 23:38:44 +01:00
|
|
|
this->deleteNode(remove(where));
|
2002-06-25 18:13:24 +02:00
|
|
|
return where;
|
|
|
|
}
|
|
|
|
|
2016-09-03 03:42:40 +02:00
|
|
|
iterator erase(pointer IT) { return erase(iterator(IT)); }
|
|
|
|
iterator erase(reference IT) { return erase(iterator(IT)); }
|
2015-10-09 20:23:49 +02:00
|
|
|
|
2013-01-04 23:35:42 +01:00
|
|
|
/// Remove all nodes from the list like clear(), but do not call
|
|
|
|
/// removeNodeFromList() or deleteNode().
|
|
|
|
///
|
|
|
|
/// This should only be used immediately before freeing nodes in bulk to
|
|
|
|
/// avoid traversing the list and bringing all the nodes into cache.
|
ADT: Split out simple_ilist, a simple intrusive list
Split out a new, low-level intrusive list type with clear semantics.
Unlike iplist (and ilist), all operations on simple_ilist are intrusive,
and simple_ilist never takes ownership of its nodes. This enables an
intuitive API that has the right defaults for intrusive lists.
- insert() takes references (not pointers!) to nodes (in iplist/ilist,
passing a reference will cause the node to be copied).
- erase() takes only iterators (like std::list), and does not destroy
the nodes.
- remove() takes only references and has the same behaviour as erase().
- clear() does not destroy the nodes.
- The destructor does not destroy the nodes.
- New API {erase,remove,clear}AndDispose() take an extra Disposer
functor for callsites that want to call some disposal routine (e.g.,
std::default_delete).
This list is not currently configurable, and has no callbacks.
The initial motivation was to fix iplist<>::sort to work correctly (even
with callbacks in ilist_traits<>). iplist<> uses simple_ilist<>::sort
directly. The new test in unittests/IR/ModuleTest.cpp crashes without
this commit.
Fixing sort() via a low-level layer provided a good opportunity to:
- Unit test the low-level functionality thoroughly.
- Modernize the API, largely inspired by other intrusive list
implementations.
Here's a sketch of a longer-term plan:
- Create BumpPtrList<>, a non-intrusive list implemented using
simple_ilist<>, and use it for the Token list in
lib/Support/YAMLParser.cpp. This will factor out the only real use of
createNode().
- Evolve the iplist<> and ilist<> APIs in the direction of
simple_ilist<>, making allocation/deallocation explicit at call sites
(similar to simple_ilist<>::eraseAndDispose()).
- Factor out remaining calls to createNode() and deleteNode() and remove
the customization from ilist_traits<>.
- Transition uses of iplist<>/ilist<> that don't need callbacks over to
simple_ilist<>.
llvm-svn: 280107
2016-08-30 18:23:55 +02:00
|
|
|
void clearAndLeakNodesUnsafely() { base_list_type::clear(); }
|
2002-06-25 18:13:24 +02:00
|
|
|
|
|
|
|
private:
|
|
|
|
// transfer - The heart of the splice function. Move linked list nodes from
|
|
|
|
// [first, last) into position.
|
|
|
|
//
|
2016-09-03 04:07:45 +02:00
|
|
|
void transfer(iterator position, iplist_impl &L2, iterator first, iterator last) {
|
Reapply "ADT: Remove UB in ilist (and use a circular linked list)"
This reverts commit r279053, reapplying r278974 after fixing PR29035
with r279104.
Note that r279312 has been committed in the meantime, and this has been
rebased on top of that. Otherwise it's identical to r278974.
Note for maintainers of out-of-tree code (that I missed in the original
message): if the new isKnownSentinel() assertion is firing from
ilist_iterator<>::operator*(), this patch has identified a bug in your
code. There are a few common patterns:
- Some IR-related APIs htake an IRUnit* that might be nullptr, and pass
in an incremented iterator as an insertion point. Some old code was
using "&*++I", which in the case of end() only worked by fluke. If
the IRUnit in question inherits from ilist_node_with_parent<>, you can
use "I->getNextNode()". Otherwise, use "List.getNextNode(*I)".
- In most other cases, crashes on &*I just need to check for I==end()
before dereferencing.
- There's also occasional code that sends iterators into a function, and
then starts calling I->getOperand() (or other API). Either check for
end() before the entering the function, or early exit.
Note for if the static_assert with HasObsoleteCustomization is firing
for you:
- r278513 has examples of how to stop using custom sentinel traits.
- r278532 removed ilist_nextprev_traits since no one was using it. See
lld's r278469 for the only migration I needed to do.
Original commit message follows.
----
This removes the undefined behaviour (UB) in ilist/ilist_node/etc.,
mainly by removing (gutting) the ilist_sentinel_traits customization
point and canonicalizing on a single, efficient memory layout. This
fixes PR26753.
The new ilist is a doubly-linked circular list.
- ilist_node_base has two ilist_node_base*: Next and Prev. Size-of: two
pointers.
- ilist_node<T> (size-of: two pointers) is a type-safe wrapper around
ilist_node_base.
- ilist_iterator<T> (size-of: two pointers) operates on an
ilist_node<T>*, and downcasts to T* on dereference.
- ilist_sentinel<T> (size-of: two pointers) is a wrapper around
ilist_node<T> that has some extra API for list management.
- ilist<T> (size-of: two pointers) has an ilist_sentinel<T>, whose
address is returned for end().
The new memory layout matches ilist_half_embedded_sentinel_traits<T>
exactly. The Head pointer that previously lived in ilist<T> is
effectively glued to the ilist_half_node<T> that lived in
ilist_half_embedded_sentinel_traits<T>, becoming the Next and Prev in
the ilist_sentinel_node<T>, respectively. sizeof(ilist<T>) is now the
size of two pointers, and there is never any additional storage for a
sentinel.
This is a much simpler design for a doubly-linked list, removing most of
the corner cases of list manipulation (add, remove, etc.). In follow-up
commits, I intend to move as many algorithms as possible into a
non-templated base class (ilist_base) to reduce code size.
Moreover, this fixes the UB in ilist_iterator/getNext/getPrev
operations. Previously, ilist_iterator<T> operated on a T*, even when
the sentinel was not of type T (i.e., ilist_embedded_sentinel_traits and
ilist_half_embedded_sentinel_traits). This added UB to all operations
involving end(). Now, ilist_iterator<T> operates on an ilist_node<T>*,
and only downcasts when the full type is guaranteed to be T*.
What did we lose? There used to be a crash (in some configurations) on
++end(). Curiously (via UB), ++end() would return begin() for users of
ilist_half_embedded_sentinel_traits<T>, but otherwise ++end() would
cause a nice dependable nullptr dereference, crashing instead of a
possible infinite loop. Options:
1. Lose that behaviour.
2. Keep it, by stealing a bit from Prev in asserts builds.
3. Crash on dereference instead, using the same technique.
Hans convinced me (because of the number of problems this and r278532
exposed on Windows) that we really need some assertion here, at least in
the short term. I've opted for #3 since I think it catches more bugs.
I added only a couple of unit tests to root out specific bugs I hit
during bring-up, but otherwise this is tested implicitly via the
extensive usage throughout LLVM.
Planned follow-ups:
- Remove ilist_*sentinel_traits<T>. Here I've just gutted them to
prevent build failures in sub-projects. Once I stop referring to them
in sub-projects, I'll come back and delete them.
- Add ilist_base and move algorithms there.
- Check and fix move construction and assignment.
Eventually, there are other interesting directions:
- Rewrite reverse iterators, so that rbegin().getNodePtr()==&*rbegin().
This allows much simpler logic when erasing elements during a reverse
traversal.
- Remove ilist_traits::createNode, by deleting the remaining API that
creates nodes. Intrusive lists shouldn't be creating nodes
themselves.
- Remove ilist_traits::deleteNode, by (1) asserting that lists are empty
on destruction and (2) changing API that calls it to take a Deleter
functor (intrusive lists shouldn't be in the memory management
business).
- Reconfigure the remaining callback traits (addNodeToList, etc.) to be
higher-level, pulling out a simple_ilist<T> that is much easier to
read and understand.
- Allow tags (e.g., ilist_node<T,tag1> and ilist_node<T,tag2>) so that T
can be a member of multiple intrusive lists.
llvm-svn: 279314
2016-08-19 22:40:12 +02:00
|
|
|
if (position == last)
|
|
|
|
return;
|
|
|
|
|
2016-08-30 20:00:45 +02:00
|
|
|
if (this != &L2) // Notify traits we moved the nodes...
|
|
|
|
this->transferNodesFromList(L2, first, last);
|
Reapply "ADT: Remove UB in ilist (and use a circular linked list)"
This reverts commit r279053, reapplying r278974 after fixing PR29035
with r279104.
Note that r279312 has been committed in the meantime, and this has been
rebased on top of that. Otherwise it's identical to r278974.
Note for maintainers of out-of-tree code (that I missed in the original
message): if the new isKnownSentinel() assertion is firing from
ilist_iterator<>::operator*(), this patch has identified a bug in your
code. There are a few common patterns:
- Some IR-related APIs htake an IRUnit* that might be nullptr, and pass
in an incremented iterator as an insertion point. Some old code was
using "&*++I", which in the case of end() only worked by fluke. If
the IRUnit in question inherits from ilist_node_with_parent<>, you can
use "I->getNextNode()". Otherwise, use "List.getNextNode(*I)".
- In most other cases, crashes on &*I just need to check for I==end()
before dereferencing.
- There's also occasional code that sends iterators into a function, and
then starts calling I->getOperand() (or other API). Either check for
end() before the entering the function, or early exit.
Note for if the static_assert with HasObsoleteCustomization is firing
for you:
- r278513 has examples of how to stop using custom sentinel traits.
- r278532 removed ilist_nextprev_traits since no one was using it. See
lld's r278469 for the only migration I needed to do.
Original commit message follows.
----
This removes the undefined behaviour (UB) in ilist/ilist_node/etc.,
mainly by removing (gutting) the ilist_sentinel_traits customization
point and canonicalizing on a single, efficient memory layout. This
fixes PR26753.
The new ilist is a doubly-linked circular list.
- ilist_node_base has two ilist_node_base*: Next and Prev. Size-of: two
pointers.
- ilist_node<T> (size-of: two pointers) is a type-safe wrapper around
ilist_node_base.
- ilist_iterator<T> (size-of: two pointers) operates on an
ilist_node<T>*, and downcasts to T* on dereference.
- ilist_sentinel<T> (size-of: two pointers) is a wrapper around
ilist_node<T> that has some extra API for list management.
- ilist<T> (size-of: two pointers) has an ilist_sentinel<T>, whose
address is returned for end().
The new memory layout matches ilist_half_embedded_sentinel_traits<T>
exactly. The Head pointer that previously lived in ilist<T> is
effectively glued to the ilist_half_node<T> that lived in
ilist_half_embedded_sentinel_traits<T>, becoming the Next and Prev in
the ilist_sentinel_node<T>, respectively. sizeof(ilist<T>) is now the
size of two pointers, and there is never any additional storage for a
sentinel.
This is a much simpler design for a doubly-linked list, removing most of
the corner cases of list manipulation (add, remove, etc.). In follow-up
commits, I intend to move as many algorithms as possible into a
non-templated base class (ilist_base) to reduce code size.
Moreover, this fixes the UB in ilist_iterator/getNext/getPrev
operations. Previously, ilist_iterator<T> operated on a T*, even when
the sentinel was not of type T (i.e., ilist_embedded_sentinel_traits and
ilist_half_embedded_sentinel_traits). This added UB to all operations
involving end(). Now, ilist_iterator<T> operates on an ilist_node<T>*,
and only downcasts when the full type is guaranteed to be T*.
What did we lose? There used to be a crash (in some configurations) on
++end(). Curiously (via UB), ++end() would return begin() for users of
ilist_half_embedded_sentinel_traits<T>, but otherwise ++end() would
cause a nice dependable nullptr dereference, crashing instead of a
possible infinite loop. Options:
1. Lose that behaviour.
2. Keep it, by stealing a bit from Prev in asserts builds.
3. Crash on dereference instead, using the same technique.
Hans convinced me (because of the number of problems this and r278532
exposed on Windows) that we really need some assertion here, at least in
the short term. I've opted for #3 since I think it catches more bugs.
I added only a couple of unit tests to root out specific bugs I hit
during bring-up, but otherwise this is tested implicitly via the
extensive usage throughout LLVM.
Planned follow-ups:
- Remove ilist_*sentinel_traits<T>. Here I've just gutted them to
prevent build failures in sub-projects. Once I stop referring to them
in sub-projects, I'll come back and delete them.
- Add ilist_base and move algorithms there.
- Check and fix move construction and assignment.
Eventually, there are other interesting directions:
- Rewrite reverse iterators, so that rbegin().getNodePtr()==&*rbegin().
This allows much simpler logic when erasing elements during a reverse
traversal.
- Remove ilist_traits::createNode, by deleting the remaining API that
creates nodes. Intrusive lists shouldn't be creating nodes
themselves.
- Remove ilist_traits::deleteNode, by (1) asserting that lists are empty
on destruction and (2) changing API that calls it to take a Deleter
functor (intrusive lists shouldn't be in the memory management
business).
- Reconfigure the remaining callback traits (addNodeToList, etc.) to be
higher-level, pulling out a simple_ilist<T> that is much easier to
read and understand.
- Allow tags (e.g., ilist_node<T,tag1> and ilist_node<T,tag2>) so that T
can be a member of multiple intrusive lists.
llvm-svn: 279314
2016-08-19 22:40:12 +02:00
|
|
|
|
2016-08-30 20:00:45 +02:00
|
|
|
base_list_type::splice(position, L2, first, last);
|
2002-06-25 18:13:24 +02:00
|
|
|
}
|
|
|
|
|
|
|
|
public:
|
|
|
|
|
|
|
|
//===----------------------------------------------------------------------===
|
|
|
|
// Functionality derived from other functions defined above...
|
|
|
|
//
|
|
|
|
|
ADT: Split out simple_ilist, a simple intrusive list
Split out a new, low-level intrusive list type with clear semantics.
Unlike iplist (and ilist), all operations on simple_ilist are intrusive,
and simple_ilist never takes ownership of its nodes. This enables an
intuitive API that has the right defaults for intrusive lists.
- insert() takes references (not pointers!) to nodes (in iplist/ilist,
passing a reference will cause the node to be copied).
- erase() takes only iterators (like std::list), and does not destroy
the nodes.
- remove() takes only references and has the same behaviour as erase().
- clear() does not destroy the nodes.
- The destructor does not destroy the nodes.
- New API {erase,remove,clear}AndDispose() take an extra Disposer
functor for callsites that want to call some disposal routine (e.g.,
std::default_delete).
This list is not currently configurable, and has no callbacks.
The initial motivation was to fix iplist<>::sort to work correctly (even
with callbacks in ilist_traits<>). iplist<> uses simple_ilist<>::sort
directly. The new test in unittests/IR/ModuleTest.cpp crashes without
this commit.
Fixing sort() via a low-level layer provided a good opportunity to:
- Unit test the low-level functionality thoroughly.
- Modernize the API, largely inspired by other intrusive list
implementations.
Here's a sketch of a longer-term plan:
- Create BumpPtrList<>, a non-intrusive list implemented using
simple_ilist<>, and use it for the Token list in
lib/Support/YAMLParser.cpp. This will factor out the only real use of
createNode().
- Evolve the iplist<> and ilist<> APIs in the direction of
simple_ilist<>, making allocation/deallocation explicit at call sites
(similar to simple_ilist<>::eraseAndDispose()).
- Factor out remaining calls to createNode() and deleteNode() and remove
the customization from ilist_traits<>.
- Transition uses of iplist<>/ilist<> that don't need callbacks over to
simple_ilist<>.
llvm-svn: 280107
2016-08-30 18:23:55 +02:00
|
|
|
using base_list_type::size;
|
2002-06-25 18:13:24 +02:00
|
|
|
|
|
|
|
iterator erase(iterator first, iterator last) {
|
|
|
|
while (first != last)
|
|
|
|
first = erase(first);
|
|
|
|
return last;
|
|
|
|
}
|
|
|
|
|
Reapply "ADT: Remove UB in ilist (and use a circular linked list)"
This reverts commit r279053, reapplying r278974 after fixing PR29035
with r279104.
Note that r279312 has been committed in the meantime, and this has been
rebased on top of that. Otherwise it's identical to r278974.
Note for maintainers of out-of-tree code (that I missed in the original
message): if the new isKnownSentinel() assertion is firing from
ilist_iterator<>::operator*(), this patch has identified a bug in your
code. There are a few common patterns:
- Some IR-related APIs htake an IRUnit* that might be nullptr, and pass
in an incremented iterator as an insertion point. Some old code was
using "&*++I", which in the case of end() only worked by fluke. If
the IRUnit in question inherits from ilist_node_with_parent<>, you can
use "I->getNextNode()". Otherwise, use "List.getNextNode(*I)".
- In most other cases, crashes on &*I just need to check for I==end()
before dereferencing.
- There's also occasional code that sends iterators into a function, and
then starts calling I->getOperand() (or other API). Either check for
end() before the entering the function, or early exit.
Note for if the static_assert with HasObsoleteCustomization is firing
for you:
- r278513 has examples of how to stop using custom sentinel traits.
- r278532 removed ilist_nextprev_traits since no one was using it. See
lld's r278469 for the only migration I needed to do.
Original commit message follows.
----
This removes the undefined behaviour (UB) in ilist/ilist_node/etc.,
mainly by removing (gutting) the ilist_sentinel_traits customization
point and canonicalizing on a single, efficient memory layout. This
fixes PR26753.
The new ilist is a doubly-linked circular list.
- ilist_node_base has two ilist_node_base*: Next and Prev. Size-of: two
pointers.
- ilist_node<T> (size-of: two pointers) is a type-safe wrapper around
ilist_node_base.
- ilist_iterator<T> (size-of: two pointers) operates on an
ilist_node<T>*, and downcasts to T* on dereference.
- ilist_sentinel<T> (size-of: two pointers) is a wrapper around
ilist_node<T> that has some extra API for list management.
- ilist<T> (size-of: two pointers) has an ilist_sentinel<T>, whose
address is returned for end().
The new memory layout matches ilist_half_embedded_sentinel_traits<T>
exactly. The Head pointer that previously lived in ilist<T> is
effectively glued to the ilist_half_node<T> that lived in
ilist_half_embedded_sentinel_traits<T>, becoming the Next and Prev in
the ilist_sentinel_node<T>, respectively. sizeof(ilist<T>) is now the
size of two pointers, and there is never any additional storage for a
sentinel.
This is a much simpler design for a doubly-linked list, removing most of
the corner cases of list manipulation (add, remove, etc.). In follow-up
commits, I intend to move as many algorithms as possible into a
non-templated base class (ilist_base) to reduce code size.
Moreover, this fixes the UB in ilist_iterator/getNext/getPrev
operations. Previously, ilist_iterator<T> operated on a T*, even when
the sentinel was not of type T (i.e., ilist_embedded_sentinel_traits and
ilist_half_embedded_sentinel_traits). This added UB to all operations
involving end(). Now, ilist_iterator<T> operates on an ilist_node<T>*,
and only downcasts when the full type is guaranteed to be T*.
What did we lose? There used to be a crash (in some configurations) on
++end(). Curiously (via UB), ++end() would return begin() for users of
ilist_half_embedded_sentinel_traits<T>, but otherwise ++end() would
cause a nice dependable nullptr dereference, crashing instead of a
possible infinite loop. Options:
1. Lose that behaviour.
2. Keep it, by stealing a bit from Prev in asserts builds.
3. Crash on dereference instead, using the same technique.
Hans convinced me (because of the number of problems this and r278532
exposed on Windows) that we really need some assertion here, at least in
the short term. I've opted for #3 since I think it catches more bugs.
I added only a couple of unit tests to root out specific bugs I hit
during bring-up, but otherwise this is tested implicitly via the
extensive usage throughout LLVM.
Planned follow-ups:
- Remove ilist_*sentinel_traits<T>. Here I've just gutted them to
prevent build failures in sub-projects. Once I stop referring to them
in sub-projects, I'll come back and delete them.
- Add ilist_base and move algorithms there.
- Check and fix move construction and assignment.
Eventually, there are other interesting directions:
- Rewrite reverse iterators, so that rbegin().getNodePtr()==&*rbegin().
This allows much simpler logic when erasing elements during a reverse
traversal.
- Remove ilist_traits::createNode, by deleting the remaining API that
creates nodes. Intrusive lists shouldn't be creating nodes
themselves.
- Remove ilist_traits::deleteNode, by (1) asserting that lists are empty
on destruction and (2) changing API that calls it to take a Deleter
functor (intrusive lists shouldn't be in the memory management
business).
- Reconfigure the remaining callback traits (addNodeToList, etc.) to be
higher-level, pulling out a simple_ilist<T> that is much easier to
read and understand.
- Allow tags (e.g., ilist_node<T,tag1> and ilist_node<T,tag2>) so that T
can be a member of multiple intrusive lists.
llvm-svn: 279314
2016-08-19 22:40:12 +02:00
|
|
|
void clear() { erase(begin(), end()); }
|
2002-06-25 18:13:24 +02:00
|
|
|
|
|
|
|
// Front and back inserters...
|
2016-09-03 03:42:40 +02:00
|
|
|
void push_front(pointer val) { insert(begin(), val); }
|
|
|
|
void push_back(pointer val) { insert(end(), val); }
|
2002-06-25 18:13:24 +02:00
|
|
|
void pop_front() {
|
|
|
|
assert(!empty() && "pop_front() on empty list!");
|
|
|
|
erase(begin());
|
|
|
|
}
|
|
|
|
void pop_back() {
|
|
|
|
assert(!empty() && "pop_back() on empty list!");
|
|
|
|
iterator t = end(); erase(--t);
|
|
|
|
}
|
|
|
|
|
|
|
|
// Special forms of insert...
|
|
|
|
template<class InIt> void insert(iterator where, InIt first, InIt last) {
|
|
|
|
for (; first != last; ++first) insert(where, *first);
|
|
|
|
}
|
|
|
|
|
|
|
|
// Splice members - defined in terms of transfer...
|
2016-09-03 04:07:45 +02:00
|
|
|
void splice(iterator where, iplist_impl &L2) {
|
2002-06-25 18:13:24 +02:00
|
|
|
if (!L2.empty())
|
|
|
|
transfer(where, L2, L2.begin(), L2.end());
|
|
|
|
}
|
2016-09-03 04:07:45 +02:00
|
|
|
void splice(iterator where, iplist_impl &L2, iterator first) {
|
2002-06-25 18:13:24 +02:00
|
|
|
iterator last = first; ++last;
|
|
|
|
if (where == first || where == last) return; // No change
|
|
|
|
transfer(where, L2, first, last);
|
|
|
|
}
|
2016-09-03 04:07:45 +02:00
|
|
|
void splice(iterator where, iplist_impl &L2, iterator first, iterator last) {
|
2002-06-25 18:13:24 +02:00
|
|
|
if (first != last) transfer(where, L2, first, last);
|
|
|
|
}
|
2016-09-03 04:07:45 +02:00
|
|
|
void splice(iterator where, iplist_impl &L2, reference N) {
|
2015-10-09 21:36:12 +02:00
|
|
|
splice(where, L2, iterator(N));
|
|
|
|
}
|
2016-09-03 04:07:45 +02:00
|
|
|
void splice(iterator where, iplist_impl &L2, pointer N) {
|
2015-10-09 21:36:12 +02:00
|
|
|
splice(where, L2, iterator(N));
|
|
|
|
}
|
2015-09-18 10:18:07 +02:00
|
|
|
|
|
|
|
template <class Compare>
|
2016-09-03 04:07:45 +02:00
|
|
|
void merge(iplist_impl &Right, Compare comp) {
|
2015-09-18 10:18:07 +02:00
|
|
|
if (this == &Right)
|
|
|
|
return;
|
ADT: Split out simple_ilist, a simple intrusive list
Split out a new, low-level intrusive list type with clear semantics.
Unlike iplist (and ilist), all operations on simple_ilist are intrusive,
and simple_ilist never takes ownership of its nodes. This enables an
intuitive API that has the right defaults for intrusive lists.
- insert() takes references (not pointers!) to nodes (in iplist/ilist,
passing a reference will cause the node to be copied).
- erase() takes only iterators (like std::list), and does not destroy
the nodes.
- remove() takes only references and has the same behaviour as erase().
- clear() does not destroy the nodes.
- The destructor does not destroy the nodes.
- New API {erase,remove,clear}AndDispose() take an extra Disposer
functor for callsites that want to call some disposal routine (e.g.,
std::default_delete).
This list is not currently configurable, and has no callbacks.
The initial motivation was to fix iplist<>::sort to work correctly (even
with callbacks in ilist_traits<>). iplist<> uses simple_ilist<>::sort
directly. The new test in unittests/IR/ModuleTest.cpp crashes without
this commit.
Fixing sort() via a low-level layer provided a good opportunity to:
- Unit test the low-level functionality thoroughly.
- Modernize the API, largely inspired by other intrusive list
implementations.
Here's a sketch of a longer-term plan:
- Create BumpPtrList<>, a non-intrusive list implemented using
simple_ilist<>, and use it for the Token list in
lib/Support/YAMLParser.cpp. This will factor out the only real use of
createNode().
- Evolve the iplist<> and ilist<> APIs in the direction of
simple_ilist<>, making allocation/deallocation explicit at call sites
(similar to simple_ilist<>::eraseAndDispose()).
- Factor out remaining calls to createNode() and deleteNode() and remove
the customization from ilist_traits<>.
- Transition uses of iplist<>/ilist<> that don't need callbacks over to
simple_ilist<>.
llvm-svn: 280107
2016-08-30 18:23:55 +02:00
|
|
|
this->transferNodesFromList(Right, Right.begin(), Right.end());
|
|
|
|
base_list_type::merge(Right, comp);
|
2015-09-18 10:18:07 +02:00
|
|
|
}
|
2016-09-03 04:07:45 +02:00
|
|
|
void merge(iplist_impl &Right) { return merge(Right, op_less); }
|
2015-09-18 10:18:07 +02:00
|
|
|
|
ADT: Split out simple_ilist, a simple intrusive list
Split out a new, low-level intrusive list type with clear semantics.
Unlike iplist (and ilist), all operations on simple_ilist are intrusive,
and simple_ilist never takes ownership of its nodes. This enables an
intuitive API that has the right defaults for intrusive lists.
- insert() takes references (not pointers!) to nodes (in iplist/ilist,
passing a reference will cause the node to be copied).
- erase() takes only iterators (like std::list), and does not destroy
the nodes.
- remove() takes only references and has the same behaviour as erase().
- clear() does not destroy the nodes.
- The destructor does not destroy the nodes.
- New API {erase,remove,clear}AndDispose() take an extra Disposer
functor for callsites that want to call some disposal routine (e.g.,
std::default_delete).
This list is not currently configurable, and has no callbacks.
The initial motivation was to fix iplist<>::sort to work correctly (even
with callbacks in ilist_traits<>). iplist<> uses simple_ilist<>::sort
directly. The new test in unittests/IR/ModuleTest.cpp crashes without
this commit.
Fixing sort() via a low-level layer provided a good opportunity to:
- Unit test the low-level functionality thoroughly.
- Modernize the API, largely inspired by other intrusive list
implementations.
Here's a sketch of a longer-term plan:
- Create BumpPtrList<>, a non-intrusive list implemented using
simple_ilist<>, and use it for the Token list in
lib/Support/YAMLParser.cpp. This will factor out the only real use of
createNode().
- Evolve the iplist<> and ilist<> APIs in the direction of
simple_ilist<>, making allocation/deallocation explicit at call sites
(similar to simple_ilist<>::eraseAndDispose()).
- Factor out remaining calls to createNode() and deleteNode() and remove
the customization from ilist_traits<>.
- Transition uses of iplist<>/ilist<> that don't need callbacks over to
simple_ilist<>.
llvm-svn: 280107
2016-08-30 18:23:55 +02:00
|
|
|
using base_list_type::sort;
|
ADT: Avoid relying on UB in ilist_node::getNextNode()
Re-implement `ilist_node::getNextNode()` and `getPrevNode()` without
relying on the sentinel having a "next" pointer. Instead, get access to
the owning list and compare against the `begin()` and `end()` iterators.
This only works when the node *can* get access to the owning list. The
new support is in `ilist_node_with_parent<>`, and any class `Ty`
inheriting from `ilist_node<NodeTy>` that wants `getNextNode()` and/or
`getPrevNode()` should inherit from
`ilist_node_with_parent<NodeTy, ParentTy>` instead. The requirements:
- `NodeTy` must have a `getParent()` function that returns the parent.
- `ParentTy` must have a `getSublistAccess()` static that, given a(n
ignored) `NodeTy*` (to determine which list), returns a member field
pointer to the appropriate `ilist<>`.
This isn't the cleanest way to get access to the owning list, but it
leverages the API already used in the IR hierarchy (see, e.g.,
`Instruction::getSublistAccess()`).
If anyone feels like ripping out the calls to `getNextNode()` and
`getPrevNode()` and replacing with direct iterator logic, they can also
remove the access function, etc., but as an incremental step, I'm
maintaining the API where it's currently used in tree.
If these requirements are *not* met, call sites with access to the ilist
can call `iplist<NodeTy>::getNextNode(NodeTy*)` directly, as in
ilistTest.cpp.
Why rewrite this?
The old code was broken, calling `getNext()` on a sentinel that possibly
didn't have a "next" pointer at all! The new code avoids that
particular flavour of UB (see the commit message for r252538 for more
details about the "lucky" memory layout that made this function so
interesting).
There's still some UB here: the end iterator gets downcast to `NodeTy*`,
even when it's a sentinel (which is typically
`ilist_half_node<NodeTy*>`). I'll tackle that in follow-up commits.
See this llvm-dev thread for more details:
http://lists.llvm.org/pipermail/llvm-dev/2015-October/091115.html
What's the danger?
There might be some code that relies on `getNextNode()` or
`getPrevNode()` *never* returning `nullptr` -- i.e., that relies on them
being broken when the sentinel is an `ilist_half_node<NodeTy>`. I tried
to root out those cases with the audits I did leading up to r252380, but
it's possible I missed one or two. I hope not.
(If (1) you have out-of-tree code, (2) you've reverted r252380
temporarily, and (3) you get some weird crashes with this commit, then I
recommend un-reverting r252380 and auditing the compile errors looking
for "strange" implicit conversions.)
llvm-svn: 252694
2015-11-11 03:26:42 +01:00
|
|
|
|
|
|
|
/// \brief Get the previous node, or \c nullptr for the list head.
|
2016-09-03 03:42:40 +02:00
|
|
|
pointer getPrevNode(reference N) const {
|
2016-02-22 21:49:58 +01:00
|
|
|
auto I = N.getIterator();
|
ADT: Avoid relying on UB in ilist_node::getNextNode()
Re-implement `ilist_node::getNextNode()` and `getPrevNode()` without
relying on the sentinel having a "next" pointer. Instead, get access to
the owning list and compare against the `begin()` and `end()` iterators.
This only works when the node *can* get access to the owning list. The
new support is in `ilist_node_with_parent<>`, and any class `Ty`
inheriting from `ilist_node<NodeTy>` that wants `getNextNode()` and/or
`getPrevNode()` should inherit from
`ilist_node_with_parent<NodeTy, ParentTy>` instead. The requirements:
- `NodeTy` must have a `getParent()` function that returns the parent.
- `ParentTy` must have a `getSublistAccess()` static that, given a(n
ignored) `NodeTy*` (to determine which list), returns a member field
pointer to the appropriate `ilist<>`.
This isn't the cleanest way to get access to the owning list, but it
leverages the API already used in the IR hierarchy (see, e.g.,
`Instruction::getSublistAccess()`).
If anyone feels like ripping out the calls to `getNextNode()` and
`getPrevNode()` and replacing with direct iterator logic, they can also
remove the access function, etc., but as an incremental step, I'm
maintaining the API where it's currently used in tree.
If these requirements are *not* met, call sites with access to the ilist
can call `iplist<NodeTy>::getNextNode(NodeTy*)` directly, as in
ilistTest.cpp.
Why rewrite this?
The old code was broken, calling `getNext()` on a sentinel that possibly
didn't have a "next" pointer at all! The new code avoids that
particular flavour of UB (see the commit message for r252538 for more
details about the "lucky" memory layout that made this function so
interesting).
There's still some UB here: the end iterator gets downcast to `NodeTy*`,
even when it's a sentinel (which is typically
`ilist_half_node<NodeTy*>`). I'll tackle that in follow-up commits.
See this llvm-dev thread for more details:
http://lists.llvm.org/pipermail/llvm-dev/2015-October/091115.html
What's the danger?
There might be some code that relies on `getNextNode()` or
`getPrevNode()` *never* returning `nullptr` -- i.e., that relies on them
being broken when the sentinel is an `ilist_half_node<NodeTy>`. I tried
to root out those cases with the audits I did leading up to r252380, but
it's possible I missed one or two. I hope not.
(If (1) you have out-of-tree code, (2) you've reverted r252380
temporarily, and (3) you get some weird crashes with this commit, then I
recommend un-reverting r252380 and auditing the compile errors looking
for "strange" implicit conversions.)
llvm-svn: 252694
2015-11-11 03:26:42 +01:00
|
|
|
if (I == begin())
|
|
|
|
return nullptr;
|
|
|
|
return &*std::prev(I);
|
|
|
|
}
|
|
|
|
/// \brief Get the previous node, or \c nullptr for the list head.
|
2016-09-03 03:42:40 +02:00
|
|
|
const_pointer getPrevNode(const_reference N) const {
|
|
|
|
return getPrevNode(const_cast<reference >(N));
|
ADT: Avoid relying on UB in ilist_node::getNextNode()
Re-implement `ilist_node::getNextNode()` and `getPrevNode()` without
relying on the sentinel having a "next" pointer. Instead, get access to
the owning list and compare against the `begin()` and `end()` iterators.
This only works when the node *can* get access to the owning list. The
new support is in `ilist_node_with_parent<>`, and any class `Ty`
inheriting from `ilist_node<NodeTy>` that wants `getNextNode()` and/or
`getPrevNode()` should inherit from
`ilist_node_with_parent<NodeTy, ParentTy>` instead. The requirements:
- `NodeTy` must have a `getParent()` function that returns the parent.
- `ParentTy` must have a `getSublistAccess()` static that, given a(n
ignored) `NodeTy*` (to determine which list), returns a member field
pointer to the appropriate `ilist<>`.
This isn't the cleanest way to get access to the owning list, but it
leverages the API already used in the IR hierarchy (see, e.g.,
`Instruction::getSublistAccess()`).
If anyone feels like ripping out the calls to `getNextNode()` and
`getPrevNode()` and replacing with direct iterator logic, they can also
remove the access function, etc., but as an incremental step, I'm
maintaining the API where it's currently used in tree.
If these requirements are *not* met, call sites with access to the ilist
can call `iplist<NodeTy>::getNextNode(NodeTy*)` directly, as in
ilistTest.cpp.
Why rewrite this?
The old code was broken, calling `getNext()` on a sentinel that possibly
didn't have a "next" pointer at all! The new code avoids that
particular flavour of UB (see the commit message for r252538 for more
details about the "lucky" memory layout that made this function so
interesting).
There's still some UB here: the end iterator gets downcast to `NodeTy*`,
even when it's a sentinel (which is typically
`ilist_half_node<NodeTy*>`). I'll tackle that in follow-up commits.
See this llvm-dev thread for more details:
http://lists.llvm.org/pipermail/llvm-dev/2015-October/091115.html
What's the danger?
There might be some code that relies on `getNextNode()` or
`getPrevNode()` *never* returning `nullptr` -- i.e., that relies on them
being broken when the sentinel is an `ilist_half_node<NodeTy>`. I tried
to root out those cases with the audits I did leading up to r252380, but
it's possible I missed one or two. I hope not.
(If (1) you have out-of-tree code, (2) you've reverted r252380
temporarily, and (3) you get some weird crashes with this commit, then I
recommend un-reverting r252380 and auditing the compile errors looking
for "strange" implicit conversions.)
llvm-svn: 252694
2015-11-11 03:26:42 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
/// \brief Get the next node, or \c nullptr for the list tail.
|
2016-09-03 03:42:40 +02:00
|
|
|
pointer getNextNode(reference N) const {
|
2016-02-22 21:49:58 +01:00
|
|
|
auto Next = std::next(N.getIterator());
|
ADT: Avoid relying on UB in ilist_node::getNextNode()
Re-implement `ilist_node::getNextNode()` and `getPrevNode()` without
relying on the sentinel having a "next" pointer. Instead, get access to
the owning list and compare against the `begin()` and `end()` iterators.
This only works when the node *can* get access to the owning list. The
new support is in `ilist_node_with_parent<>`, and any class `Ty`
inheriting from `ilist_node<NodeTy>` that wants `getNextNode()` and/or
`getPrevNode()` should inherit from
`ilist_node_with_parent<NodeTy, ParentTy>` instead. The requirements:
- `NodeTy` must have a `getParent()` function that returns the parent.
- `ParentTy` must have a `getSublistAccess()` static that, given a(n
ignored) `NodeTy*` (to determine which list), returns a member field
pointer to the appropriate `ilist<>`.
This isn't the cleanest way to get access to the owning list, but it
leverages the API already used in the IR hierarchy (see, e.g.,
`Instruction::getSublistAccess()`).
If anyone feels like ripping out the calls to `getNextNode()` and
`getPrevNode()` and replacing with direct iterator logic, they can also
remove the access function, etc., but as an incremental step, I'm
maintaining the API where it's currently used in tree.
If these requirements are *not* met, call sites with access to the ilist
can call `iplist<NodeTy>::getNextNode(NodeTy*)` directly, as in
ilistTest.cpp.
Why rewrite this?
The old code was broken, calling `getNext()` on a sentinel that possibly
didn't have a "next" pointer at all! The new code avoids that
particular flavour of UB (see the commit message for r252538 for more
details about the "lucky" memory layout that made this function so
interesting).
There's still some UB here: the end iterator gets downcast to `NodeTy*`,
even when it's a sentinel (which is typically
`ilist_half_node<NodeTy*>`). I'll tackle that in follow-up commits.
See this llvm-dev thread for more details:
http://lists.llvm.org/pipermail/llvm-dev/2015-October/091115.html
What's the danger?
There might be some code that relies on `getNextNode()` or
`getPrevNode()` *never* returning `nullptr` -- i.e., that relies on them
being broken when the sentinel is an `ilist_half_node<NodeTy>`. I tried
to root out those cases with the audits I did leading up to r252380, but
it's possible I missed one or two. I hope not.
(If (1) you have out-of-tree code, (2) you've reverted r252380
temporarily, and (3) you get some weird crashes with this commit, then I
recommend un-reverting r252380 and auditing the compile errors looking
for "strange" implicit conversions.)
llvm-svn: 252694
2015-11-11 03:26:42 +01:00
|
|
|
if (Next == end())
|
|
|
|
return nullptr;
|
|
|
|
return &*Next;
|
|
|
|
}
|
|
|
|
/// \brief Get the next node, or \c nullptr for the list tail.
|
2016-09-03 03:42:40 +02:00
|
|
|
const_pointer getNextNode(const_reference N) const {
|
|
|
|
return getNextNode(const_cast<reference >(N));
|
ADT: Avoid relying on UB in ilist_node::getNextNode()
Re-implement `ilist_node::getNextNode()` and `getPrevNode()` without
relying on the sentinel having a "next" pointer. Instead, get access to
the owning list and compare against the `begin()` and `end()` iterators.
This only works when the node *can* get access to the owning list. The
new support is in `ilist_node_with_parent<>`, and any class `Ty`
inheriting from `ilist_node<NodeTy>` that wants `getNextNode()` and/or
`getPrevNode()` should inherit from
`ilist_node_with_parent<NodeTy, ParentTy>` instead. The requirements:
- `NodeTy` must have a `getParent()` function that returns the parent.
- `ParentTy` must have a `getSublistAccess()` static that, given a(n
ignored) `NodeTy*` (to determine which list), returns a member field
pointer to the appropriate `ilist<>`.
This isn't the cleanest way to get access to the owning list, but it
leverages the API already used in the IR hierarchy (see, e.g.,
`Instruction::getSublistAccess()`).
If anyone feels like ripping out the calls to `getNextNode()` and
`getPrevNode()` and replacing with direct iterator logic, they can also
remove the access function, etc., but as an incremental step, I'm
maintaining the API where it's currently used in tree.
If these requirements are *not* met, call sites with access to the ilist
can call `iplist<NodeTy>::getNextNode(NodeTy*)` directly, as in
ilistTest.cpp.
Why rewrite this?
The old code was broken, calling `getNext()` on a sentinel that possibly
didn't have a "next" pointer at all! The new code avoids that
particular flavour of UB (see the commit message for r252538 for more
details about the "lucky" memory layout that made this function so
interesting).
There's still some UB here: the end iterator gets downcast to `NodeTy*`,
even when it's a sentinel (which is typically
`ilist_half_node<NodeTy*>`). I'll tackle that in follow-up commits.
See this llvm-dev thread for more details:
http://lists.llvm.org/pipermail/llvm-dev/2015-October/091115.html
What's the danger?
There might be some code that relies on `getNextNode()` or
`getPrevNode()` *never* returning `nullptr` -- i.e., that relies on them
being broken when the sentinel is an `ilist_half_node<NodeTy>`. I tried
to root out those cases with the audits I did leading up to r252380, but
it's possible I missed one or two. I hope not.
(If (1) you have out-of-tree code, (2) you've reverted r252380
temporarily, and (3) you get some weird crashes with this commit, then I
recommend un-reverting r252380 and auditing the compile errors looking
for "strange" implicit conversions.)
llvm-svn: 252694
2015-11-11 03:26:42 +01:00
|
|
|
}
|
2002-06-25 18:13:24 +02:00
|
|
|
};
|
|
|
|
|
2016-09-03 04:07:45 +02:00
|
|
|
/// An intrusive list with ownership and callbacks specified/controlled by
|
|
|
|
/// ilist_traits, only with API safe for polymorphic types.
|
2016-09-11 18:20:53 +02:00
|
|
|
///
|
|
|
|
/// The \p Options parameters are the same as those for \a simple_ilist. See
|
|
|
|
/// there for a description of what's available.
|
|
|
|
template <class T, class... Options>
|
|
|
|
class iplist
|
2016-09-12 00:11:37 +02:00
|
|
|
: public iplist_impl<simple_ilist<T, Options...>, ilist_traits<T>> {
|
|
|
|
typedef typename iplist::iplist_impl_type iplist_impl_type;
|
|
|
|
|
|
|
|
public:
|
|
|
|
iplist() = default;
|
|
|
|
iplist(iplist &&X) : iplist_impl_type(std::move(X)) {}
|
|
|
|
iplist(const iplist &X) = delete;
|
|
|
|
iplist &operator=(iplist &&X) {
|
|
|
|
*static_cast<iplist_impl_type *>(this) = std::move(X);
|
|
|
|
return *this;
|
|
|
|
}
|
|
|
|
iplist &operator=(const iplist &X) = delete;
|
|
|
|
};
|
2016-09-03 04:07:45 +02:00
|
|
|
|
|
|
|
/// An intrusive list with ownership and callbacks specified/controlled by
|
|
|
|
/// ilist_traits, with API that is unsafe for polymorphic types.
|
2016-09-11 18:20:53 +02:00
|
|
|
template <class T, class... Options>
|
|
|
|
class ilist : public iplist<T, Options...> {
|
|
|
|
typedef iplist<T, Options...> base_list_type;
|
2002-06-25 18:13:24 +02:00
|
|
|
|
2016-09-03 03:42:40 +02:00
|
|
|
public:
|
|
|
|
typedef typename base_list_type::size_type size_type;
|
|
|
|
typedef typename base_list_type::iterator iterator;
|
|
|
|
typedef typename base_list_type::value_type value_type;
|
|
|
|
typedef typename base_list_type::const_reference const_reference;
|
2002-07-24 22:44:01 +02:00
|
|
|
|
2002-06-25 18:13:24 +02:00
|
|
|
ilist() {}
|
2016-09-03 03:42:40 +02:00
|
|
|
ilist(const ilist &right) : base_list_type() {
|
2003-08-29 16:22:29 +02:00
|
|
|
insert(this->begin(), right.begin(), right.end());
|
2002-06-25 18:13:24 +02:00
|
|
|
}
|
|
|
|
explicit ilist(size_type count) {
|
2016-09-03 03:42:40 +02:00
|
|
|
insert(this->begin(), count, value_type());
|
2009-01-08 03:21:23 +01:00
|
|
|
}
|
2016-09-03 03:42:40 +02:00
|
|
|
ilist(size_type count, const_reference val) {
|
2003-08-29 16:22:29 +02:00
|
|
|
insert(this->begin(), count, val);
|
2002-06-25 18:13:24 +02:00
|
|
|
}
|
|
|
|
template<class InIt> ilist(InIt first, InIt last) {
|
2003-08-29 16:22:29 +02:00
|
|
|
insert(this->begin(), first, last);
|
2002-06-25 18:13:24 +02:00
|
|
|
}
|
|
|
|
|
2016-09-12 00:11:37 +02:00
|
|
|
ilist(ilist &&X) : base_list_type(std::move(X)) {}
|
|
|
|
ilist &operator=(ilist &&X) {
|
|
|
|
*static_cast<base_list_type *>(this) = std::move(X);
|
|
|
|
return *this;
|
|
|
|
}
|
|
|
|
|
2009-03-02 20:49:29 +01:00
|
|
|
// bring hidden functions into scope
|
2016-09-03 03:42:40 +02:00
|
|
|
using base_list_type::insert;
|
|
|
|
using base_list_type::push_front;
|
|
|
|
using base_list_type::push_back;
|
2002-06-25 18:13:24 +02:00
|
|
|
|
|
|
|
// Main implementation here - Insert for a node passed by value...
|
2016-09-03 03:42:40 +02:00
|
|
|
iterator insert(iterator where, const_reference val) {
|
2009-12-15 04:10:26 +01:00
|
|
|
return insert(where, this->createNode(val));
|
2002-06-25 18:13:24 +02:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
// Front and back inserters...
|
2016-09-03 03:42:40 +02:00
|
|
|
void push_front(const_reference val) { insert(this->begin(), val); }
|
|
|
|
void push_back(const_reference val) { insert(this->end(), val); }
|
2002-06-25 18:13:24 +02:00
|
|
|
|
2016-09-03 03:42:40 +02:00
|
|
|
void insert(iterator where, size_type count, const_reference val) {
|
2002-06-25 18:13:24 +02:00
|
|
|
for (; count != 0; --count) insert(where, val);
|
|
|
|
}
|
|
|
|
|
|
|
|
// Assign special forms...
|
2016-09-03 03:42:40 +02:00
|
|
|
void assign(size_type count, const_reference val) {
|
2003-08-29 16:22:29 +02:00
|
|
|
iterator I = this->begin();
|
|
|
|
for (; I != this->end() && count != 0; ++I, --count)
|
2002-06-25 18:13:24 +02:00
|
|
|
*I = val;
|
|
|
|
if (count != 0)
|
2003-08-29 16:22:29 +02:00
|
|
|
insert(this->end(), val, val);
|
2002-06-25 18:13:24 +02:00
|
|
|
else
|
2003-08-29 16:22:29 +02:00
|
|
|
erase(I, this->end());
|
2002-06-25 18:13:24 +02:00
|
|
|
}
|
2003-08-29 16:22:29 +02:00
|
|
|
template<class InIt> void assign(InIt first1, InIt last1) {
|
|
|
|
iterator first2 = this->begin(), last2 = this->end();
|
2002-06-25 18:13:24 +02:00
|
|
|
for ( ; first1 != last1 && first2 != last2; ++first1, ++first2)
|
|
|
|
*first1 = *first2;
|
|
|
|
if (first2 == last2)
|
|
|
|
erase(first1, last1);
|
|
|
|
else
|
|
|
|
insert(last1, first2, last2);
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
// Resize members...
|
2016-09-03 03:42:40 +02:00
|
|
|
void resize(size_type newsize, value_type val) {
|
2003-08-29 16:22:29 +02:00
|
|
|
iterator i = this->begin();
|
2002-06-25 18:13:24 +02:00
|
|
|
size_type len = 0;
|
2003-08-29 16:22:29 +02:00
|
|
|
for ( ; i != this->end() && len < newsize; ++i, ++len) /* empty*/ ;
|
2002-06-25 18:13:24 +02:00
|
|
|
|
|
|
|
if (len == newsize)
|
2003-08-29 16:22:29 +02:00
|
|
|
erase(i, this->end());
|
2002-06-25 18:13:24 +02:00
|
|
|
else // i == end()
|
2003-08-29 16:22:29 +02:00
|
|
|
insert(this->end(), newsize - len, val);
|
2002-06-25 18:13:24 +02:00
|
|
|
}
|
2016-09-03 03:42:40 +02:00
|
|
|
void resize(size_type newsize) { resize(newsize, value_type()); }
|
2002-06-25 18:13:24 +02:00
|
|
|
};
|
|
|
|
|
2015-06-23 11:49:53 +02:00
|
|
|
} // End llvm namespace
|
2003-11-11 23:41:34 +01:00
|
|
|
|
2002-06-25 18:13:24 +02:00
|
|
|
namespace std {
|
|
|
|
// Ensure that swap uses the fast list swap...
|
|
|
|
template<class Ty>
|
2003-11-11 23:41:34 +01:00
|
|
|
void swap(llvm::iplist<Ty> &Left, llvm::iplist<Ty> &Right) {
|
2002-06-25 18:13:24 +02:00
|
|
|
Left.swap(Right);
|
|
|
|
}
|
|
|
|
} // End 'std' extensions...
|
|
|
|
|
2008-05-29 20:17:53 +02:00
|
|
|
#endif // LLVM_ADT_ILIST_H
|