Fix getParent / getNodeBaseParent to return pointer to const (#94224)

[dklimkin]

Fix getParent / getNodeBaseParent to return pointer to const as opposed to const pointer.

This is particularly problematic as the returned object iterators get const bit unset.

[llvmbot]

@llvm/pr-subscribers-llvm-adt

Author: Danial Klimkin (dklimkin)

Changes

Fix getParent / getNodeBaseParent to return pointer to const as opposed to const pointer.

This is particularly problematic as the returned object iterators get const bit unset.

---

Full diff: https://github.com/llvm/llvm-project/pull/96053.diff

2 Files Affected:

• (modified) llvm/include/llvm/ADT/ilist_node.h (+4-1)
• (modified) llvm/include/llvm/ADT/ilist_node_base.h (+4-1)

diff --git a/llvm/include/llvm/ADT/ilist_node.h b/llvm/include/llvm/ADT/ilist_node.h
index 209fd209167e8..db0319d487433 100644
--- a/llvm/include/llvm/ADT/ilist_node.h
+++ b/llvm/include/llvm/ADT/ilist_node.h
@@ -17,6 +17,7 @@
 
 #include "llvm/ADT/ilist_node_base.h"
 #include "llvm/ADT/ilist_node_options.h"
+#include <type_traits>
 
 namespace llvm {
 
@@ -29,7 +30,9 @@ struct NodeAccess;
 /// has been set in the list options.
 template <class NodeTy, class ParentPtrTy> class node_parent_access {
 public:
-  inline const ParentPtrTy getParent() const {
+  using ConstParentPtrTy =
+      std::add_pointer_t<std::add_const_t<std::remove_pointer_t<ParentPtrTy>>>;
+  inline ConstParentPtrTy getParent() const {
     return static_cast<const NodeTy *>(this)->getNodeBaseParent();
   }
   inline ParentPtrTy getParent() {
diff --git a/llvm/include/llvm/ADT/ilist_node_base.h b/llvm/include/llvm/ADT/ilist_node_base.h
index caad87cdd4d71..5a8244336188f 100644
--- a/llvm/include/llvm/ADT/ilist_node_base.h
+++ b/llvm/include/llvm/ADT/ilist_node_base.h
@@ -10,6 +10,7 @@
 #define LLVM_ADT_ILIST_NODE_BASE_H
 
 #include "llvm/ADT/PointerIntPair.h"
+#include <type_traits>
 
 namespace llvm {
 
@@ -48,10 +49,12 @@ template <class NodeBase> class node_base_prevnext<NodeBase, true> {
 
 template <class ParentPtrTy> class node_base_parent {
   ParentPtrTy Parent = nullptr;
+  using ConstParentPtrTy =
+      std::add_pointer_t<std::add_const_t<std::remove_pointer_t<ParentPtrTy>>>;
 
 public:
   void setNodeBaseParent(ParentPtrTy Parent) { this->Parent = Parent; }
-  inline const ParentPtrTy getNodeBaseParent() const { return Parent; }
+  inline ConstParentPtrTy getNodeBaseParent() const { return Parent; }
   inline ParentPtrTy getNodeBaseParent() { return Parent; }
 };
 template <> class node_base_parent<void> {};

[github-actions]

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

[dklimkin]

Ideally, I'd prefer ParentPtrTy to be ParentTy and used as ParentTy* throughout but it's a much larger change.

[SLTozer]

Ideally, I'd prefer ParentPtrTy to be ParentTy and used as ParentTy* throughout but it's a much larger change.

I just commented exactly that on the original PR - I originally passed around ParentPtrTy to preserve the possibility of a void* parent pointer since it seemed like a 0-cost decision that could possibly be useful in future, but since it now results in messier template logic I'm fine to drop it.

This patch LGTM, so if you need to merge it to unblock something downstream then go ahead; if it's not so urgent, then I'll upload a patch that fixes it as described above instead.

[dklimkin]

I've put an internal fix for this now so we are good to wait for the patch and reduce churn. Thank you.