Skip to content

[ADT] Fix incorrect const parent ptr type in ilist #96059

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 19, 2024
Merged

[ADT] Fix incorrect const parent ptr type in ilist #96059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b727ac3
[ADT] Fix incorrect const parent ptr type in ilist
SLTozer Jun 19, 2024
c7ef898
Add unittests for const behaviour
SLTozer Jun 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions llvm/include/llvm/ADT/ilist_base.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
26 changes: 13 additions & 13 deletions llvm/include/llvm/ADT/ilist_iterator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
};
Expand All @@ -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>;
Expand Down Expand Up @@ -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>;
Expand Down
18 changes: 9 additions & 9 deletions llvm/include/llvm/ADT/ilist_node.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
};
Expand Down Expand Up @@ -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;
Expand All @@ -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>;
Expand Down
21 changes: 10 additions & 11 deletions llvm/include/llvm/ADT/ilist_node_base.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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> {};

Expand All @@ -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

Expand Down
15 changes: 7 additions & 8 deletions llvm/include/llvm/ADT/ilist_node_options.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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...> {};
Expand All @@ -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;
Expand All @@ -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 {
Expand Down
10 changes: 10 additions & 0 deletions llvm/unittests/ADT/IListIteratorTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
17 changes: 14 additions & 3 deletions llvm/unittests/ADT/IListNodeBaseTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include "llvm/ADT/ilist_node_base.h"
#include "gtest/gtest.h"

#include <type_traits>

using namespace llvm;

namespace {
Expand All @@ -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;
Expand Down Expand Up @@ -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());
Expand All @@ -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
6 changes: 3 additions & 3 deletions llvm/unittests/ADT/IListNodeTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
Loading