[llvm] [ADT] Fix incorrect const parent ptr type in ilist (PR #96059)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 19 04:31:28 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-adt
Author: Stephen Tozer (SLTozer)
<details>
<summary>Changes</summary>
Fixes issue reported in: https://github.com/llvm/llvm-project/pull/94224
The recent commit above added an ilist_parent<ParentTy> option, which added a parent pointer to the ilist_node_base type for the list. The const methods for returning that parent pointer however were incorrectly implemented, returning `const ParentPtrTy`, which is equivalent to `ParentTy * const` rather than `const ParentTy *`. This patch fixes this by passing around `ParentTy` in ilist's internal logic rather than `ParentPtrTy`, removing the ability to have a `void*` parent pointer but cleanly fixing this error.
---
Full diff: https://github.com/llvm/llvm-project/pull/96059.diff
7 Files Affected:
- (modified) llvm/include/llvm/ADT/ilist_base.h (+2-2)
- (modified) llvm/include/llvm/ADT/ilist_iterator.h (+13-13)
- (modified) llvm/include/llvm/ADT/ilist_node.h (+9-9)
- (modified) llvm/include/llvm/ADT/ilist_node_base.h (+10-11)
- (modified) llvm/include/llvm/ADT/ilist_node_options.h (+7-8)
- (modified) llvm/unittests/ADT/IListNodeBaseTest.cpp (+2-2)
- (modified) llvm/unittests/ADT/IListNodeTest.cpp (+3-3)
``````````diff
diff --git a/llvm/include/llvm/ADT/ilist_base.h b/llvm/include/llvm/ADT/ilist_base.h
index 000253a26012b..4a8a556fc99be 100644
--- a/llvm/include/llvm/ADT/ilist_base.h
+++ b/llvm/include/llvm/ADT/ilist_base.h
@@ -15,9 +15,9 @@
namespace llvm {
/// Implementations of list algorithms using ilist_node_base.
-template <bool EnableSentinelTracking, class ParentPtrTy> class ilist_base {
+template <bool EnableSentinelTracking, class ParentTy> class ilist_base {
public:
- using node_base_type = ilist_node_base<EnableSentinelTracking, ParentPtrTy>;
+ using node_base_type = ilist_node_base<EnableSentinelTracking, ParentTy>;
static void insertBeforeImpl(node_base_type &Next, node_base_type &N) {
node_base_type &Prev = *Next.getPrev();
diff --git a/llvm/include/llvm/ADT/ilist_iterator.h b/llvm/include/llvm/ADT/ilist_iterator.h
index 16a45b5beac50..882df9d7e767f 100644
--- a/llvm/include/llvm/ADT/ilist_iterator.h
+++ b/llvm/include/llvm/ADT/ilist_iterator.h
@@ -53,19 +53,19 @@ template <> struct IteratorHelper<true> : ilist_detail::NodeAccess {
/// Mixin class used to add a \a getNodeParent() function to iterators iff the
/// list uses \a ilist_parent, calling through to the node's \a getParent(). For
/// more details see \a ilist_node.
-template <class IteratorTy, class ParentPtrTy, bool IsConst>
+template <class IteratorTy, class ParentTy, bool IsConst>
class iterator_parent_access;
-template <class IteratorTy, class ParentPtrTy>
-class iterator_parent_access<IteratorTy, ParentPtrTy, true> {
+template <class IteratorTy, class ParentTy>
+class iterator_parent_access<IteratorTy, ParentTy, true> {
public:
- inline const ParentPtrTy getNodeParent() const {
+ inline const ParentTy *getNodeParent() const {
return static_cast<IteratorTy *>(this)->NodePtr->getParent();
}
};
-template <class IteratorTy, class ParentPtrTy>
-class iterator_parent_access<IteratorTy, ParentPtrTy, false> {
+template <class IteratorTy, class ParentTy>
+class iterator_parent_access<IteratorTy, ParentTy, false> {
public:
- inline ParentPtrTy getNodeParent() {
+ inline ParentTy *getNodeParent() {
return static_cast<IteratorTy *>(this)->NodePtr->getParent();
}
};
@@ -81,13 +81,13 @@ template <class OptionsT, bool IsReverse, bool IsConst>
class ilist_iterator : ilist_detail::SpecificNodeAccess<OptionsT>,
public ilist_detail::iterator_parent_access<
ilist_iterator<OptionsT, IsReverse, IsConst>,
- typename OptionsT::parent_ptr_ty, IsConst> {
+ typename OptionsT::parent_ty, IsConst> {
friend ilist_iterator<OptionsT, IsReverse, !IsConst>;
friend ilist_iterator<OptionsT, !IsReverse, IsConst>;
friend ilist_iterator<OptionsT, !IsReverse, !IsConst>;
friend ilist_detail::iterator_parent_access<
- ilist_iterator<OptionsT, IsReverse, IsConst>,
- typename OptionsT::parent_ptr_ty, IsConst>;
+ ilist_iterator<OptionsT, IsReverse, IsConst>,
+ typename OptionsT::parent_ty, IsConst>;
using Traits = ilist_detail::IteratorTraits<OptionsT, IsConst>;
using Access = ilist_detail::SpecificNodeAccess<OptionsT>;
@@ -215,13 +215,13 @@ class ilist_iterator_w_bits
: ilist_detail::SpecificNodeAccess<OptionsT>,
public ilist_detail::iterator_parent_access<
ilist_iterator_w_bits<OptionsT, IsReverse, IsConst>,
- typename OptionsT::parent_ptr_ty, IsConst> {
+ typename OptionsT::parent_ty, IsConst> {
friend ilist_iterator_w_bits<OptionsT, IsReverse, !IsConst>;
friend ilist_iterator_w_bits<OptionsT, !IsReverse, IsConst>;
friend ilist_iterator<OptionsT, !IsReverse, !IsConst>;
friend ilist_detail::iterator_parent_access<
- ilist_iterator_w_bits<OptionsT, IsReverse, IsConst>,
- typename OptionsT::parent_ptr_ty, IsConst>;
+ ilist_iterator_w_bits<OptionsT, IsReverse, IsConst>,
+ typename OptionsT::parent_ty, IsConst>;
using Traits = ilist_detail::IteratorTraits<OptionsT, IsConst>;
using Access = ilist_detail::SpecificNodeAccess<OptionsT>;
diff --git a/llvm/include/llvm/ADT/ilist_node.h b/llvm/include/llvm/ADT/ilist_node.h
index 209fd209167e8..bfd50f8b3fb71 100644
--- a/llvm/include/llvm/ADT/ilist_node.h
+++ b/llvm/include/llvm/ADT/ilist_node.h
@@ -25,17 +25,17 @@ namespace ilist_detail {
struct NodeAccess;
/// Mixin base class that is used to add \a getParent() and
-/// \a setParent(ParentPtrTy) methods to \a ilist_node_impl iff \a ilist_parent
+/// \a setParent(ParentTy*) methods to \a ilist_node_impl iff \a ilist_parent
/// has been set in the list options.
-template <class NodeTy, class ParentPtrTy> class node_parent_access {
+template <class NodeTy, class ParentTy> class node_parent_access {
public:
- inline const ParentPtrTy getParent() const {
+ inline const ParentTy *getParent() const {
return static_cast<const NodeTy *>(this)->getNodeBaseParent();
}
- inline ParentPtrTy getParent() {
+ inline ParentTy *getParent() {
return static_cast<NodeTy *>(this)->getNodeBaseParent();
}
- void setParent(ParentPtrTy Parent) {
+ void setParent(ParentTy *Parent) {
return static_cast<NodeTy *>(this)->setNodeBaseParent(Parent);
}
};
@@ -71,8 +71,8 @@ class ilist_select_iterator_type<true, Opts, arg1, arg2> {
template <class OptionsT>
class ilist_node_impl
: OptionsT::node_base_type,
- public ilist_detail::node_parent_access<
- ilist_node_impl<OptionsT>, typename OptionsT::parent_ptr_ty> {
+ public ilist_detail::node_parent_access<ilist_node_impl<OptionsT>,
+ typename OptionsT::parent_ty> {
using value_type = typename OptionsT::value_type;
using node_base_type = typename OptionsT::node_base_type;
using list_base_type = typename OptionsT::list_base_type;
@@ -81,8 +81,8 @@ class ilist_node_impl
friend struct ilist_detail::NodeAccess;
friend class ilist_sentinel<OptionsT>;
- friend class ilist_detail::node_parent_access<
- ilist_node_impl<OptionsT>, typename OptionsT::parent_ptr_ty>;
+ friend class ilist_detail::node_parent_access<ilist_node_impl<OptionsT>,
+ typename OptionsT::parent_ty>;
friend class ilist_iterator<OptionsT, false, false>;
friend class ilist_iterator<OptionsT, false, true>;
friend class ilist_iterator<OptionsT, true, false>;
diff --git a/llvm/include/llvm/ADT/ilist_node_base.h b/llvm/include/llvm/ADT/ilist_node_base.h
index caad87cdd4d71..49b197d3466d9 100644
--- a/llvm/include/llvm/ADT/ilist_node_base.h
+++ b/llvm/include/llvm/ADT/ilist_node_base.h
@@ -46,13 +46,13 @@ template <class NodeBase> class node_base_prevnext<NodeBase, true> {
void initializeSentinel() { PrevAndSentinel.setInt(true); }
};
-template <class ParentPtrTy> class node_base_parent {
- ParentPtrTy Parent = nullptr;
+template <class ParentTy> class node_base_parent {
+ ParentTy *Parent = nullptr;
public:
- void setNodeBaseParent(ParentPtrTy Parent) { this->Parent = Parent; }
- inline const ParentPtrTy getNodeBaseParent() const { return Parent; }
- inline ParentPtrTy getNodeBaseParent() { return Parent; }
+ void setNodeBaseParent(ParentTy *Parent) { this->Parent = Parent; }
+ inline const ParentTy *getNodeBaseParent() const { return Parent; }
+ inline ParentTy *getNodeBaseParent() { return Parent; }
};
template <> class node_base_parent<void> {};
@@ -61,12 +61,11 @@ template <> class node_base_parent<void> {};
/// Base class for ilist nodes.
///
/// Optionally tracks whether this node is the sentinel.
-template <bool EnableSentinelTracking, class ParentPtrTy>
-class ilist_node_base
- : public ilist_detail::node_base_prevnext<
- ilist_node_base<EnableSentinelTracking, ParentPtrTy>,
- EnableSentinelTracking>,
- public ilist_detail::node_base_parent<ParentPtrTy> {};
+template <bool EnableSentinelTracking, class ParentTy>
+class ilist_node_base : public ilist_detail::node_base_prevnext<
+ ilist_node_base<EnableSentinelTracking, ParentTy>,
+ EnableSentinelTracking>,
+ public ilist_detail::node_base_parent<ParentTy> {};
} // end namespace llvm
diff --git a/llvm/include/llvm/ADT/ilist_node_options.h b/llvm/include/llvm/ADT/ilist_node_options.h
index aa32162cd51e4..d26e79b925ad1 100644
--- a/llvm/include/llvm/ADT/ilist_node_options.h
+++ b/llvm/include/llvm/ADT/ilist_node_options.h
@@ -15,8 +15,8 @@
namespace llvm {
-template <bool EnableSentinelTracking, class ParentPtrTy> class ilist_node_base;
-template <bool EnableSentinelTracking, class ParentPtrTy> class ilist_base;
+template <bool EnableSentinelTracking, class ParentTy> class ilist_node_base;
+template <bool EnableSentinelTracking, class ParentTy> class ilist_base;
/// Option to choose whether to track sentinels.
///
@@ -134,7 +134,7 @@ struct is_valid_option<ilist_iterator_bits<IteratorBits>> : std::true_type {};
template <class... Options> struct extract_parent;
template <class ParentTy, class... Options>
struct extract_parent<ilist_parent<ParentTy>, Options...> {
- typedef ParentTy *type;
+ typedef ParentTy type;
};
template <class Option1, class... Options>
struct extract_parent<Option1, Options...> : extract_parent<Options...> {};
@@ -156,7 +156,7 @@ struct check_options<Option1, Options...>
///
/// This is usually computed via \a compute_node_options.
template <class T, bool EnableSentinelTracking, bool IsSentinelTrackingExplicit,
- class TagT, bool HasIteratorBits, class ParentPtrTy>
+ class TagT, bool HasIteratorBits, class ParentTy>
struct node_options {
typedef T value_type;
typedef T *pointer;
@@ -168,10 +168,9 @@ struct node_options {
static const bool is_sentinel_tracking_explicit = IsSentinelTrackingExplicit;
static const bool has_iterator_bits = HasIteratorBits;
typedef TagT tag;
- typedef ParentPtrTy parent_ptr_ty;
- typedef ilist_node_base<enable_sentinel_tracking, parent_ptr_ty>
- node_base_type;
- typedef ilist_base<enable_sentinel_tracking, parent_ptr_ty> list_base_type;
+ typedef ParentTy parent_ty;
+ typedef ilist_node_base<enable_sentinel_tracking, parent_ty> node_base_type;
+ typedef ilist_base<enable_sentinel_tracking, parent_ty> list_base_type;
};
template <class T, class... Options> struct compute_node_options {
diff --git a/llvm/unittests/ADT/IListNodeBaseTest.cpp b/llvm/unittests/ADT/IListNodeBaseTest.cpp
index 07f397d2ef0fe..2d44e320d42ed 100644
--- a/llvm/unittests/ADT/IListNodeBaseTest.cpp
+++ b/llvm/unittests/ADT/IListNodeBaseTest.cpp
@@ -17,8 +17,8 @@ class Parent {};
typedef ilist_node_base<false, void> RawNode;
typedef ilist_node_base<true, void> TrackingNode;
-typedef ilist_node_base<false, Parent*> ParentNode;
-typedef ilist_node_base<true, Parent*> ParentTrackingNode;
+typedef ilist_node_base<false, Parent> ParentNode;
+typedef ilist_node_base<true, Parent> ParentTrackingNode;
TEST(IListNodeBaseTest, DefaultConstructor) {
RawNode A;
diff --git a/llvm/unittests/ADT/IListNodeTest.cpp b/llvm/unittests/ADT/IListNodeTest.cpp
index 0a5da12d7f30f..c5f39d7eb8d8d 100644
--- a/llvm/unittests/ADT/IListNodeTest.cpp
+++ b/llvm/unittests/ADT/IListNodeTest.cpp
@@ -66,9 +66,9 @@ TEST(IListNodeTest, Options) {
ilist_sentinel_tracking<true>>::type>,
"order shouldn't matter with real tags");
static_assert(
- !std::is_same_v<compute_node_options<Node>::type,
- compute_node_options<Node, ilist_parent<void>>::type>,
- "void parent is different to no parent");
+ std::is_same_v<compute_node_options<Node>::type,
+ compute_node_options<Node, ilist_parent<void>>::type>,
+ "default parent is void");
static_assert(
!std::is_same_v<compute_node_options<Node, ilist_parent<ParentA>>::type,
compute_node_options<Node, ilist_parent<void>>::type>,
``````````
</details>
https://github.com/llvm/llvm-project/pull/96059
More information about the llvm-commits
mailing list