1
0
mirror of https://github.com/RPCS3/llvm-mirror.git synced 2024-10-19 11:02:59 +02:00
llvm-mirror/include/llvm/ADT/ilist.h

404 lines
13 KiB
C
Raw Normal View History

//==-- llvm/ADT/ilist.h - Intrusive Linked List Template ---------*- C++ -*-==//
//
2003-10-20 21:46:57 +02:00
// The LLVM Compiler Infrastructure
//
// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.
//
2003-10-20 21:46:57 +02:00
//===----------------------------------------------------------------------===//
//
// This file defines classes to implement an intrusive doubly linked list class
// (i.e. each node of the list must contain a next and previous field for the
// list.
//
// 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.
//
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().
//
//===----------------------------------------------------------------------===//
2008-05-29 20:17:53 +02:00
#ifndef LLVM_ADT_ILIST_H
#define LLVM_ADT_ILIST_H
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"
#include "llvm/Support/Compiler.h"
#include <cassert>
#include <cstddef>
#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>
namespace llvm {
/// A fragment for template traits for intrusive list that provides default
/// node related operations.
///
/// TODO: Split up (alloc vs. callback) and delete.
template <typename NodeTy> struct ilist_node_traits {
static NodeTy *createNode(const NodeTy &V) { return new NodeTy(V); }
static void deleteNode(NodeTy *V) { delete V; }
void addNodeToList(NodeTy *) {}
void removeNodeFromList(NodeTy *) {}
void transferNodesFromList(ilist_node_traits & /*SrcTraits*/,
ilist_iterator<NodeTy> /*first*/,
ilist_iterator<NodeTy> /*last*/) {}
};
/// 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> {};
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).
template <class TraitsT, class NodeT> struct HasGetNext {
typedef char Yes[1];
typedef char No[2];
template <size_t N> struct SFINAE {};
template <class U>
static Yes &test(U *I, decltype(I->getNext(&make<NodeT>())) * = 0);
template <class> static No &test(...);
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];
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(...);
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);
};
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;
};
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
//===----------------------------------------------------------------------===//
//
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 subset of list functionality that can safely be used on nodes of
/// 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.
template <typename NodeTy, typename Traits = ilist_traits<NodeTy>>
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
class iplist : public Traits, simple_ilist<NodeTy> {
// 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).
static_assert(!ilist_detail::HasObsoleteCustomization<Traits, NodeTy>::value,
"ilist customization points have changed!");
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 simple_ilist<NodeTy> base_list_type;
static bool op_less(NodeTy &L, NodeTy &R) { return L < R; }
static bool op_equal(NodeTy &L, NodeTy &R) { return L == R; }
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
// Copying intrusively linked nodes doesn't make sense.
iplist(const iplist &) = delete;
void operator=(const iplist &) = delete;
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;
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
iplist() = default;
~iplist() { clear(); }
// Miscellaneous inspection routines.
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;
void swap(iplist &RHS) {
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);
}
iterator insert(iterator where, NodeTy *New) {
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
auto I = base_list_type::insert(where, *New);
this->addNodeToList(New); // Notify traits that we added a node...
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
return I;
}
iterator insert(iterator where, const NodeTy &New) {
return this->insert(where, new NodeTy(New));
}
iterator insertAfter(iterator where, NodeTy *New) {
if (empty())
return insert(begin(), New);
else
return insert(++where, New);
}
NodeTy *remove(iterator &IT) {
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
NodeTy *Node = &*IT;
base_list_type::erase(IT++);
this->removeNodeFromList(Node); // Notify traits that we removed a node...
return Node;
}
NodeTy *remove(const iterator &IT) {
iterator MutIt = IT;
return remove(MutIt);
}
NodeTy *remove(NodeTy *IT) { return remove(iterator(IT)); }
NodeTy *remove(NodeTy &IT) { return remove(iterator(IT)); }
// erase - remove a node from the controlled sequence... and delete it.
iterator erase(iterator where) {
this->deleteNode(remove(where));
return where;
}
iterator erase(NodeTy *IT) { return erase(iterator(IT)); }
iterator erase(NodeTy &IT) { return erase(iterator(IT)); }
/// 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(); }
private:
// transfer - The heart of the splice function. Move linked list nodes from
// [first, last) into position.
//
void transfer(iterator position, iplist &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;
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::splice(position, 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
// Callback. Note that the nodes have moved from before-last to
// before-position.
this->transferNodesFromList(L2, first, position);
}
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;
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()); }
// Front and back inserters...
void push_front(NodeTy *val) { insert(begin(), val); }
void push_back(NodeTy *val) { insert(end(), val); }
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...
void splice(iterator where, iplist &L2) {
if (!L2.empty())
transfer(where, L2, L2.begin(), L2.end());
}
void splice(iterator where, iplist &L2, iterator first) {
iterator last = first; ++last;
if (where == first || where == last) return; // No change
transfer(where, L2, first, last);
}
void splice(iterator where, iplist &L2, iterator first, iterator last) {
if (first != last) transfer(where, L2, first, last);
}
void splice(iterator where, iplist &L2, NodeTy &N) {
splice(where, L2, iterator(N));
}
void splice(iterator where, iplist &L2, NodeTy *N) {
splice(where, L2, iterator(N));
}
template <class Compare>
void merge(iplist &Right, Compare comp) {
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);
}
void merge(iplist &Right) { return merge(Right, op_less); }
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.
NodeTy *getPrevNode(NodeTy &N) const {
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.
const NodeTy *getPrevNode(const NodeTy &N) const {
return getPrevNode(const_cast<NodeTy &>(N));
}
/// \brief Get the next node, or \c nullptr for the list tail.
NodeTy *getNextNode(NodeTy &N) const {
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.
const NodeTy *getNextNode(const NodeTy &N) const {
return getNextNode(const_cast<NodeTy &>(N));
}
};
template<typename NodeTy>
struct ilist : public iplist<NodeTy> {
typedef typename iplist<NodeTy>::size_type size_type;
typedef typename iplist<NodeTy>::iterator iterator;
ilist() {}
ilist(const ilist &right) : iplist<NodeTy>() {
insert(this->begin(), right.begin(), right.end());
}
explicit ilist(size_type count) {
insert(this->begin(), count, NodeTy());
}
ilist(size_type count, const NodeTy &val) {
insert(this->begin(), count, val);
}
template<class InIt> ilist(InIt first, InIt last) {
insert(this->begin(), first, last);
}
// bring hidden functions into scope
using iplist<NodeTy>::insert;
using iplist<NodeTy>::push_front;
using iplist<NodeTy>::push_back;
// Main implementation here - Insert for a node passed by value...
iterator insert(iterator where, const NodeTy &val) {
return insert(where, this->createNode(val));
}
// Front and back inserters...
void push_front(const NodeTy &val) { insert(this->begin(), val); }
void push_back(const NodeTy &val) { insert(this->end(), val); }
void insert(iterator where, size_type count, const NodeTy &val) {
for (; count != 0; --count) insert(where, val);
}
// Assign special forms...
void assign(size_type count, const NodeTy &val) {
iterator I = this->begin();
for (; I != this->end() && count != 0; ++I, --count)
*I = val;
if (count != 0)
insert(this->end(), val, val);
else
erase(I, this->end());
}
template<class InIt> void assign(InIt first1, InIt last1) {
iterator first2 = this->begin(), last2 = this->end();
for ( ; first1 != last1 && first2 != last2; ++first1, ++first2)
*first1 = *first2;
if (first2 == last2)
erase(first1, last1);
else
insert(last1, first2, last2);
}
// Resize members...
void resize(size_type newsize, NodeTy val) {
iterator i = this->begin();
size_type len = 0;
for ( ; i != this->end() && len < newsize; ++i, ++len) /* empty*/ ;
if (len == newsize)
erase(i, this->end());
else // i == end()
insert(this->end(), newsize - len, val);
}
void resize(size_type newsize) { resize(newsize, NodeTy()); }
};
} // End llvm namespace
namespace std {
// Ensure that swap uses the fast list swap...
template<class Ty>
void swap(llvm::iplist<Ty> &Left, llvm::iplist<Ty> &Right) {
Left.swap(Right);
}
} // End 'std' extensions...
2008-05-29 20:17:53 +02:00
#endif // LLVM_ADT_ILIST_H