diff options
author | Stephen Tozer <stephen.tozer@sony.com> | 2024-06-19 19:52:40 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-06-19 19:52:40 +0100 |
commit | c6ed8289b7c948464855841632f6b6783da1b65a (patch) | |
tree | b302a33ec1302bf755fafa1982900ccb5a0c55a2 /llvm | |
parent | 02af67c88f393cd6998949cc1bf8075553579a42 (diff) | |
download | llvm-c6ed8289b7c948464855841632f6b6783da1b65a.zip llvm-c6ed8289b7c948464855841632f6b6783da1b65a.tar.gz llvm-c6ed8289b7c948464855841632f6b6783da1b65a.tar.bz2 |
[ADT] Fix incorrect const parent ptr type in ilist (#96059)
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.
Diffstat (limited to 'llvm')
-rw-r--r-- | llvm/include/llvm/ADT/ilist_base.h | 4 | ||||
-rw-r--r-- | llvm/include/llvm/ADT/ilist_iterator.h | 26 | ||||
-rw-r--r-- | llvm/include/llvm/ADT/ilist_node.h | 18 | ||||
-rw-r--r-- | llvm/include/llvm/ADT/ilist_node_base.h | 21 | ||||
-rw-r--r-- | llvm/include/llvm/ADT/ilist_node_options.h | 15 | ||||
-rw-r--r-- | llvm/unittests/ADT/IListIteratorTest.cpp | 10 | ||||
-rw-r--r-- | llvm/unittests/ADT/IListNodeBaseTest.cpp | 17 | ||||
-rw-r--r-- | llvm/unittests/ADT/IListNodeTest.cpp | 6 |
8 files changed, 68 insertions, 49 deletions
diff --git a/llvm/include/llvm/ADT/ilist_base.h b/llvm/include/llvm/ADT/ilist_base.h index 000253a..4a8a556 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 16a45b5..882df9d 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 209fd20..bfd50f8b 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 @@ public: 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 caad87c..49b197d 100644 --- a/llvm/include/llvm/ADT/ilist_node_base.h +++ b/llvm/include/llvm/ADT/ilist_node_base.h @@ -46,13 +46,13 @@ public: 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 aa32162..d26e79b 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/IListIteratorTest.cpp b/llvm/unittests/ADT/IListIteratorTest.cpp index fd420b9..4e5b847 100644 --- a/llvm/unittests/ADT/IListIteratorTest.cpp +++ b/llvm/unittests/ADT/IListIteratorTest.cpp @@ -175,6 +175,7 @@ TEST(IListIteratorTest, GetParent) { simple_ilist<ParentNode, ilist_parent<Parent>> L; Parent P; ParentNode A; + const simple_ilist<ParentNode, ilist_parent<Parent>> &CL = L; // Parents are not set automatically. A.setParent(&P); @@ -187,6 +188,15 @@ TEST(IListIteratorTest, GetParent) { EXPECT_EQ(&P, L.end().getNodeParent()); EXPECT_EQ(&P, L.rbegin().getNodeParent()); EXPECT_EQ(&P, L.rend().getNodeParent()); + + using VarParentTy = + std::remove_pointer_t<decltype(L.begin().getNodeParent())>; + using ConstParentTy = + std::remove_pointer_t<decltype(CL.begin().getNodeParent())>; + static_assert( + std::is_const_v<ConstParentTy> && + std::is_same_v<VarParentTy, std::remove_const_t<ConstParentTy>>, + "`getNodeParent() const` adds const to parent type"); } } // end namespace diff --git a/llvm/unittests/ADT/IListNodeBaseTest.cpp b/llvm/unittests/ADT/IListNodeBaseTest.cpp index 07f397d..ef90c71 100644 --- a/llvm/unittests/ADT/IListNodeBaseTest.cpp +++ b/llvm/unittests/ADT/IListNodeBaseTest.cpp @@ -9,6 +9,8 @@ #include "llvm/ADT/ilist_node_base.h" #include "gtest/gtest.h" +#include <type_traits> + using namespace llvm; namespace { @@ -17,8 +19,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; @@ -177,9 +179,10 @@ TEST(IListNodeBaseTest, isKnownSentinel) { EXPECT_EQ(&PTB, PTA.getNext()); } -TEST(IListNodeBaseTest, setNodeBaseParent) { +TEST(IListNodeBaseTest, nodeBaseParent) { Parent Par; ParentNode PA; + const ParentNode CPA; EXPECT_EQ(nullptr, PA.getNodeBaseParent()); PA.setNodeBaseParent(&Par); EXPECT_EQ(&Par, PA.getNodeBaseParent()); @@ -188,6 +191,14 @@ TEST(IListNodeBaseTest, setNodeBaseParent) { EXPECT_EQ(nullptr, PTA.getNodeBaseParent()); PTA.setNodeBaseParent(&Par); EXPECT_EQ(&Par, PTA.getNodeBaseParent()); + + using VarParentTy = std::remove_pointer_t<decltype(PA.getNodeBaseParent())>; + using ConstParentTy = + std::remove_pointer_t<decltype(CPA.getNodeBaseParent())>; + static_assert( + std::is_const_v<ConstParentTy> && + std::is_same_v<VarParentTy, std::remove_const_t<ConstParentTy>>, + "`getNodeBaseParent() const` adds const to parent type"); } } // end namespace diff --git a/llvm/unittests/ADT/IListNodeTest.cpp b/llvm/unittests/ADT/IListNodeTest.cpp index 0a5da12..c5f39d7 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>, |