From 1728e0f5a00252ba744c9dbfdf90375a0a44da6b Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Mon, 9 Dec 2024 11:33:02 +0800 Subject: [PATCH 1/3] [libc++][ranges] Fix `ranges::to` with ADL-only `begin`/`end` When implementing the resolution of LWG4016, the `ranges::for_each` call to reduce inclusion dependency. However, the inlining was not always correct, because builtin-in range-`for` may just stop and fail when a deleted member `begin`/`end` function is encountered, while `ranges` CPOs continue to consider ADL-found `begin`/`end`. As a result, we should still use `ranges::begin`/`ranges::end`. --- libcxx/include/__ranges/to.h | 7 +++++-- .../range.utility.conv/to.pass.cpp | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/libcxx/include/__ranges/to.h b/libcxx/include/__ranges/to.h index c937b0656de87..360c33ee8f208 100644 --- a/libcxx/include/__ranges/to.h +++ b/libcxx/include/__ranges/to.h @@ -109,8 +109,11 @@ template __result.reserve(static_cast>(ranges::size(__range))); } - for (auto&& __ref : __range) { - using _Ref = decltype(__ref); + auto __iter = ranges::begin(__range); + auto __sent = ranges::end(__range); + for (; __iter != __sent; ++__iter) { + auto&& __ref = *__iter; + using _Ref = decltype(__ref); if constexpr (requires { __result.emplace_back(std::declval<_Ref>()); }) { __result.emplace_back(std::forward<_Ref>(__ref)); } else if constexpr (requires { __result.push_back(std::declval<_Ref>()); }) { diff --git a/libcxx/test/std/ranges/range.utility/range.utility.conv/to.pass.cpp b/libcxx/test/std/ranges/range.utility/range.utility.conv/to.pass.cpp index a983745fd636e..26c7af6d88479 100644 --- a/libcxx/test/std/ranges/range.utility/range.utility.conv/to.pass.cpp +++ b/libcxx/test/std/ranges/range.utility/range.utility.conv/to.pass.cpp @@ -550,11 +550,30 @@ constexpr void test_recursive() { assert(std::ranges::to(in_owning_view) == result); } +struct adl_only_range { + static constexpr int numbers[2]{42, 1729}; + + void begin() const = delete; + void end() const = delete; + + friend constexpr const int* begin(const adl_only_range&) { return std::ranges::begin(numbers); } + friend constexpr const int* end(const adl_only_range&) { return std::ranges::end(numbers); } +}; + +constexpr void test_lwg4016_regression() { + using Cont = Container; + + std::ranges::contiguous_range auto r = adl_only_range{}; + auto v = r | std::ranges::to(); + assert(std::ranges::equal(v, adl_only_range::numbers)); +} + constexpr bool test() { test_constraints(); test_ctr_choice_order(); test_lwg_3785(); test_recursive(); + test_lwg4016_regression(); return true; } From ea3a23fc67422b9dfdb78bc9078202aa489bd8cd Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Mon, 3 Mar 2025 00:01:44 +0800 Subject: [PATCH 2/3] Switch to use `ranges::for_each` --- libcxx/include/__ranges/to.h | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/libcxx/include/__ranges/to.h b/libcxx/include/__ranges/to.h index 360c33ee8f208..1aeea4256d74d 100644 --- a/libcxx/include/__ranges/to.h +++ b/libcxx/include/__ranges/to.h @@ -10,6 +10,7 @@ #ifndef _LIBCPP___RANGES_TO_H #define _LIBCPP___RANGES_TO_H +#include <__algorithm/ranges_for_each.h> #include <__concepts/constructible.h> #include <__concepts/convertible_to.h> #include <__concepts/derived_from.h> @@ -70,6 +71,22 @@ concept __constructible_from_iter_pair = derived_from>::iterator_category, input_iterator_tag> && constructible_from<_Container, iterator_t<_Range>, sentinel_t<_Range>, _Args...>; +template +constexpr auto __container_append(_Container& __c) { + return [&__c](_Ref&& __ref) { + if constexpr (requires { __c.emplace_back(std::declval<_Ref>()); }) + __c.emplace_back(std::forward<_Ref>(__ref)); + else if constexpr (requires { __c.push_back(std::declval<_Ref>()); }) + __c.push_back(std::forward<_Ref>(__ref)); + else if constexpr (requires { __c.emplace(__c.end(), std::declval<_Ref>()); }) + __c.emplace(__c.end(), std::forward<_Ref>(__ref)); + else { + static_assert(requires { __c.insert(__c.end(), std::declval<_Ref>()); }); + __c.insert(__c.end(), std::forward<_Ref>(__ref)); + } + }; +} + template concept __always_false = false; @@ -109,24 +126,8 @@ template __result.reserve(static_cast>(ranges::size(__range))); } - auto __iter = ranges::begin(__range); - auto __sent = ranges::end(__range); - for (; __iter != __sent; ++__iter) { - auto&& __ref = *__iter; - using _Ref = decltype(__ref); - if constexpr (requires { __result.emplace_back(std::declval<_Ref>()); }) { - __result.emplace_back(std::forward<_Ref>(__ref)); - } else if constexpr (requires { __result.push_back(std::declval<_Ref>()); }) { - __result.push_back(std::forward<_Ref>(__ref)); - } else if constexpr (requires { __result.emplace(__result.end(), std::declval<_Ref>()); }) { - __result.emplace(__result.end(), std::forward<_Ref>(__ref)); - } else { - static_assert(requires { __result.insert(__result.end(), std::declval<_Ref>()); }); - __result.insert(__result.end(), std::forward<_Ref>(__ref)); - } - } + ranges::for_each(__range, ranges::__container_append(__result)); return __result; - } else { static_assert(__always_false<_Container>, "ranges::to: unable to convert to the given container type."); } From 883ca4f849a410db9b6c0b23dc729c92d08a48d5 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Mon, 3 Mar 2025 00:37:20 +0800 Subject: [PATCH 3/3] Workaround for lacking implementation of P0588R1 --- libcxx/include/__ranges/to.h | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/libcxx/include/__ranges/to.h b/libcxx/include/__ranges/to.h index 1aeea4256d74d..2ef6606595526 100644 --- a/libcxx/include/__ranges/to.h +++ b/libcxx/include/__ranges/to.h @@ -71,20 +71,18 @@ concept __constructible_from_iter_pair = derived_from>::iterator_category, input_iterator_tag> && constructible_from<_Container, iterator_t<_Range>, sentinel_t<_Range>, _Args...>; -template -constexpr auto __container_append(_Container& __c) { - return [&__c](_Ref&& __ref) { - if constexpr (requires { __c.emplace_back(std::declval<_Ref>()); }) - __c.emplace_back(std::forward<_Ref>(__ref)); - else if constexpr (requires { __c.push_back(std::declval<_Ref>()); }) - __c.push_back(std::forward<_Ref>(__ref)); - else if constexpr (requires { __c.emplace(__c.end(), std::declval<_Ref>()); }) - __c.emplace(__c.end(), std::forward<_Ref>(__ref)); - else { - static_assert(requires { __c.insert(__c.end(), std::declval<_Ref>()); }); - __c.insert(__c.end(), std::forward<_Ref>(__ref)); - } - }; +template +[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr auto __container_append(_Container& __c) { + if constexpr (requires { __c.emplace_back(std::declval<_Ref>()); }) + return [&__c](_Ref&& __ref) { __c.emplace_back(std::forward<_Ref>(__ref)); }; + else if constexpr (requires { __c.push_back(std::declval<_Ref>()); }) + return [&__c](_Ref&& __ref) { __c.push_back(std::forward<_Ref>(__ref)); }; + else if constexpr (requires { __c.emplace(__c.end(), std::declval<_Ref>()); }) + return [&__c](_Ref&& __ref) { __c.emplace(__c.end(), std::forward<_Ref>(__ref)); }; + else { + static_assert(requires { __c.insert(__c.end(), std::declval<_Ref>()); }); + return [&__c](_Ref&& __ref) { __c.insert(__c.end(), std::forward<_Ref>(__ref)); }; + } } template @@ -126,7 +124,7 @@ template __result.reserve(static_cast>(ranges::size(__range))); } - ranges::for_each(__range, ranges::__container_append(__result)); + ranges::for_each(__range, ranges::__container_append>(__result)); return __result; } else { static_assert(__always_false<_Container>, "ranges::to: unable to convert to the given container type.");