Skip to content

[libc++] Make forward_list constexpr as part of P3372R3 #129435

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 8 commits into from
Jun 11, 2025
Merged

[libc++] Make forward_list constexpr as part of P3372R3 #129435

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
838c50f
Make forward_list constexpr as part of P3372R3
winner245 Feb 28, 2025
3d213b1
Address reviewers comments
winner245 Mar 2, 2025
b6fd6de
Fix constant evaluation due to non-constant pointer_traits::pointer_t…
winner245 Mar 3, 2025
a5b63dd
Revert to libc++ pointer_traits::pointer_to with necessary cast
winner245 Mar 4, 2025
7c90175
Add __static_fancy_pointer_cast
winner245 Mar 4, 2025
6b7800a
Fix ADL call
winner245 Mar 4, 2025
30daee8
Rebase onto main to resolve conflicts
winner245 May 8, 2025
1b3743e
Merge branch 'main' into constexpr-forward_list
ldionne Jun 11, 2025
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
2 changes: 2 additions & 0 deletions libcxx/docs/FeatureTestMacroTable.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,8 @@ Status
---------------------------------------------------------- -----------------
``__cpp_lib_constexpr_algorithms`` ``202306L``
---------------------------------------------------------- -----------------
``__cpp_lib_constexpr_forward_list`` ``202502L``
---------------------------------------------------------- -----------------
``__cpp_lib_constexpr_new`` ``202406L``
---------------------------------------------------------- -----------------
``__cpp_lib_constexpr_queue`` ``202502L``
Expand Down
20 changes: 11 additions & 9 deletions libcxx/include/__memory/allocation_guard.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,24 +49,26 @@ struct __allocation_guard {
using _Size _LIBCPP_NODEBUG = typename allocator_traits<_Alloc>::size_type;

template <class _AllocT> // we perform the allocator conversion inside the constructor
_LIBCPP_HIDE_FROM_ABI explicit __allocation_guard(_AllocT __alloc, _Size __n)
_LIBCPP_CONSTEXPR_SINCE_CXX26 _LIBCPP_HIDE_FROM_ABI explicit __allocation_guard(_AllocT __alloc, _Size __n)
: __alloc_(std::move(__alloc)),
__n_(__n),
__ptr_(allocator_traits<_Alloc>::allocate(__alloc_, __n_)) // initialization order is important
{}

_LIBCPP_HIDE_FROM_ABI ~__allocation_guard() _NOEXCEPT { __destroy(); }
_LIBCPP_CONSTEXPR_SINCE_CXX26 _LIBCPP_HIDE_FROM_ABI ~__allocation_guard() _NOEXCEPT { __destroy(); }

_LIBCPP_HIDE_FROM_ABI __allocation_guard(const __allocation_guard&) = delete;
_LIBCPP_HIDE_FROM_ABI __allocation_guard(__allocation_guard&& __other) _NOEXCEPT
__allocation_guard(const __allocation_guard&) = delete;
__allocation_guard& operator=(const __allocation_guard& __other) = delete;

_LIBCPP_CONSTEXPR_SINCE_CXX26 _LIBCPP_HIDE_FROM_ABI __allocation_guard(__allocation_guard&& __other) _NOEXCEPT
: __alloc_(std::move(__other.__alloc_)),
__n_(__other.__n_),
__ptr_(__other.__ptr_) {
__other.__ptr_ = nullptr;
}

_LIBCPP_HIDE_FROM_ABI __allocation_guard& operator=(const __allocation_guard& __other) = delete;
_LIBCPP_HIDE_FROM_ABI __allocation_guard& operator=(__allocation_guard&& __other) _NOEXCEPT {
_LIBCPP_CONSTEXPR_SINCE_CXX26 _LIBCPP_HIDE_FROM_ABI __allocation_guard&
operator=(__allocation_guard&& __other) _NOEXCEPT {
if (std::addressof(__other) != this) {
__destroy();

Expand All @@ -79,17 +81,17 @@ struct __allocation_guard {
return *this;
}

_LIBCPP_HIDE_FROM_ABI _Pointer
_LIBCPP_CONSTEXPR_SINCE_CXX26 _LIBCPP_HIDE_FROM_ABI _Pointer
__release_ptr() _NOEXCEPT { // not called __release() because it's a keyword in objective-c++
_Pointer __tmp = __ptr_;
__ptr_ = nullptr;
return __tmp;
}

_LIBCPP_HIDE_FROM_ABI _Pointer __get() const _NOEXCEPT { return __ptr_; }
_LIBCPP_CONSTEXPR_SINCE_CXX26 _LIBCPP_HIDE_FROM_ABI _Pointer __get() const _NOEXCEPT { return __ptr_; }

private:
_LIBCPP_HIDE_FROM_ABI void __destroy() _NOEXCEPT {
_LIBCPP_CONSTEXPR_SINCE_CXX26 _LIBCPP_HIDE_FROM_ABI void __destroy() _NOEXCEPT {
if (__ptr_ != nullptr) {
allocator_traits<_Alloc>::deallocate(__alloc_, __ptr_, __n_);
}
Expand Down
16 changes: 14 additions & 2 deletions libcxx/include/__memory/pointer_traits.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,8 @@ inline _LIBCPP_HIDE_FROM_ABI constexpr auto to_address(_Tp* __p) noexcept {
}

template <class _Pointer>
inline _LIBCPP_HIDE_FROM_ABI constexpr auto
to_address(const _Pointer& __p) noexcept -> decltype(std::__to_address(__p)) {
inline _LIBCPP_HIDE_FROM_ABI constexpr auto to_address(const _Pointer& __p) noexcept
-> decltype(std::__to_address(__p)) {
return std::__to_address(__p);
}
#endif
Expand Down Expand Up @@ -302,6 +302,18 @@ concept __resettable_smart_pointer_with_args = requires(_Smart __s, _Pointer __p

#endif

// This function ensures safe conversions between fancy pointers at compile-time, where we avoid casts from/to
// `__void_pointer` by obtaining the underlying raw pointer from the fancy pointer using `std::to_address`,
// then dereferencing it to retrieve the pointed-to object, and finally constructing the target fancy pointer
// to that object using the `std::pointer_traits<>::pinter_to` function.
template <class _PtrTo, class _PtrFrom>
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI _PtrTo __static_fancy_pointer_cast(const _PtrFrom& __p) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you document what this function does with a short comment?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we can't replace the cases like we did in list?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you referring to #130310 ? If so, that PR (which hasn't landed yet) removes many instances of these casts, but not all of them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess so. I think my main question is whether this utility is still needed after that patch. If not, I think I'd like this patch to be rebased on the other instead of adding a utility we know for a fact is removed by a follow-up patch. That would simplify this patch, which I think is quite important. The "constexpr everything" patches are really difficult to go through due to all the changes (even though none of them are complicated), so every bit of simplification makes it significantly easier to look at.

Copy link
Contributor Author

@winner245 winner245 Jun 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what #130310 really does is to inline the three cast functions __as_iter_node, __next_as_begin, __get_unsafe_node_pointer, which cast between __node_pointer and __begin_node_pointer via __void_pointer. During runtime evaluation, the intermediate __void_pointer is unnecessary. During compile time evaluation, it leads to compilation failure to use __void_pointer to cast between fancy pointers (e.g., min_pointer), which is why the new utility function __static_fancy_pointer_cast was introduced in this patch. __static_fancy_pointer_cast works by first extracting the underlying raw pointer from the fancy pointer using std::to_address, then dereferencing it to obtain the pointed-to object, and finally acquiring the target fancy pointer to that object. This ensures successful compile-time evaluation for fancy pointers.

That said, I’d like to land this patch first to guarantee that all fancy pointer-related casts function correctly. Once that’s done, #130310 can rebase and serve as a follow-up to inline the three cast functions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When using the full definition of min_pointer your code works correctly: https://godbolt.org/z/bEW8n6d5P (except that you're calling a private constructor)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I intentionally changed that private constructor to public in my demo, in order to mimic the use of that private constructor in min_allocator (which is a friend of min_pointer). When every test for forward_list involving min_allocator/min_pointer works fine in runtime evaluation, some tests fail due to the invalid cast at compile-time evaluation, as shown in my demo: https://godbolt.org/z/Keb5jhWYz

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw that. My point was that with the full definition of min_allocator things seem to work fine - without any instantiation problems.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe @winner245 's point is that if you have a min_pointer actually pointing to an object (as opposed to nullptr), then the cast fails in constexpr. For example, this is @philnik777's previous godbolt example modified to point to an actual object instead of default initializing the min_pointer (which requires making a private constructor public but that's besides the point): https://godbolt.org/z/dYbjz36Ec

Given this, I believe it is reasonable to unblock this patch landing and clean it up slightly after landing #130310 -- that's also the author's preference in terms of ease-of-rebasing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I didn't see that. OTOH that error is entirely unrelated to the error in the original link.

Given the claim that #130310 doesn't remove the need for the helper I do think that that's worth blocking over though. This is something we should look into before introducing a questionable workaround.

using __ptr_traits = pointer_traits<_PtrTo>;
using __element_type = typename __ptr_traits::element_type;
return __p ? __ptr_traits::pointer_to(*static_cast<__element_type*>(std::addressof(*__p)))
: static_cast<_PtrTo>(nullptr);
}

_LIBCPP_END_NAMESPACE_STD

_LIBCPP_POP_MACROS
Expand Down
Loading
Loading