[libc++] Remove unnecessary static_casts in std::forward_list

[winner245]

The patch removes unnecessary casts to void* pointers and eliminates an identity cast.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Peng Liu (winner245)

Changes

The patch removes unnecessary casts to void* pointers and eliminates an identity cast.

---

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

1 Files Affected:

• (modified) libcxx/include/forward_list (+5-13)

diff --git a/libcxx/include/forward_list b/libcxx/include/forward_list
index 7582de20995b9..1d1415aaa0e27 100644
--- a/libcxx/include/forward_list
+++ b/libcxx/include/forward_list
@@ -296,7 +296,7 @@ struct __forward_node_traits {
 #  endif
 
   _LIBCPP_HIDE_FROM_ABI static __begin_node_pointer __as_iter_node(__node_pointer __p) {
-    return static_cast<__begin_node_pointer>(static_cast<__void_pointer>(__p));
+    return static_cast<__begin_node_pointer>(__p);
   }
 };
 
@@ -363,12 +363,8 @@ class _LIBCPP_TEMPLATE_VIS __forward_list_iterator {
 
   __begin_node_pointer __ptr_;
 
-  _LIBCPP_HIDE_FROM_ABI __begin_node_pointer __get_begin() const {
-    return static_cast<__begin_node_pointer>(static_cast<__void_pointer>(__ptr_));
-  }
-  _LIBCPP_HIDE_FROM_ABI __node_pointer __get_unsafe_node_pointer() const {
-    return static_cast<__node_pointer>(static_cast<__void_pointer>(__ptr_));
-  }
+  _LIBCPP_HIDE_FROM_ABI __begin_node_pointer __get_begin() const { return __ptr_; }
+  _LIBCPP_HIDE_FROM_ABI __node_pointer __get_unsafe_node_pointer() const { return static_cast<__node_pointer>(__ptr_); }
 
   _LIBCPP_HIDE_FROM_ABI explicit __forward_list_iterator(nullptr_t) _NOEXCEPT : __ptr_(nullptr) {}
 
@@ -427,12 +423,8 @@ class _LIBCPP_TEMPLATE_VIS __forward_list_const_iterator {
 
   __begin_node_pointer __ptr_;
 
-  _LIBCPP_HIDE_FROM_ABI __begin_node_pointer __get_begin() const {
-    return static_cast<__begin_node_pointer>(static_cast<__void_pointer>(__ptr_));
-  }
-  _LIBCPP_HIDE_FROM_ABI __node_pointer __get_unsafe_node_pointer() const {
-    return static_cast<__node_pointer>(static_cast<__void_pointer>(__ptr_));
-  }
+  _LIBCPP_HIDE_FROM_ABI __begin_node_pointer __get_begin() const { return __ptr_; }
+  _LIBCPP_HIDE_FROM_ABI __node_pointer __get_unsafe_node_pointer() const { return static_cast<__node_pointer>(__ptr_); }
 
   _LIBCPP_HIDE_FROM_ABI explicit __forward_list_const_iterator(nullptr_t) _NOEXCEPT : __ptr_(nullptr) {}
 

[philnik777]

@nico Could you maybe check whether you see any breakage with this change?

[philnik777]

I think we should just inline this. That probably makes the code easier to read, since you don't have to guess what __as_iter_node means.

[winner245]

Yeah, I agree that inlining these short one-liner static_casts makes sense—it doesn’t bloat the code and actually improves readability. For instance, after getting rid of the unnecessary casts, __get_begin() is nothing but a class getter, and inlining _iter.__get_begin() as __iter.__ptr_ makes the code more obvious. So I've inlined all three functions. Additionally, I noticed that __p->__next_as_begin() exactly duplicates the functionality of __as_iter_node(__p->__next_), so I think we can get rid of the function __next_as_begin() entirely. Then we can inline the calls to __next_as_begin() as well (if we choose not to inline __as_iter_node, we can simply replace the __next_as_begin calls by__as_iter_node). I'm not sure if I am doing too much of inlining here (please let me know if you dislike some of the inlining).

[philnik777]

These probably as well, but I'm a bit less sure about them.

[philnik777]

LGTM. @ldionne do you have any thoughts?

[ldionne]

LGTM

[ldionne]

@nico Could you maybe check whether you see any breakage with this change?

Is there a reason why you suspect this might break some code?

[philnik777]

@nico Could you maybe check whether you see any breakage with this change?

Is there a reason why you suspect this might break some code?

Nothing specific. It's mostly that this requires a converting constructor that it didn't before and I wasn't able to find any specifics about fancy pointers in the standard.

[ldionne]

Ack. I think this looks fine to me, but we might want to double-check what @frederick-vs-ja thinks about this one. Frederick, I'm also unfamiliar with the requirements on pointer-like types in the standard, and http://eel.is/c++draft/memory is very light on details. Do you know if we're allowed to assume that we can convert between fancy pointer types via conversions that would be legal if we were using raw pointers instead? Such as fancy_pointer<Derived> to fancy_pointer<Base>.