[libcxx-commits] [libcxx] [libc++][ranges] Fix `ranges::to` with ADL-only `begin`/`end` (PR #119161)

A. Jiang via libcxx-commits libcxx-commits at lists.llvm.org
Sun Dec 8 19:34:14 PST 2024


https://github.com/frederick-vs-ja created https://github.com/llvm/llvm-project/pull/119161

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`.

Follows-up #113103. Fixes #119133.

>From 1728e0f5a00252ba744c9dbfdf90375a0a44da6b Mon Sep 17 00:00:00 2001
From: "A. Jiang" <de34 at live.cn>
Date: Mon, 9 Dec 2024 11:33:02 +0800
Subject: [PATCH] [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 c937b0656de87d..360c33ee8f2083 100644
--- a/libcxx/include/__ranges/to.h
+++ b/libcxx/include/__ranges/to.h
@@ -109,8 +109,11 @@ template <class _Container, input_range _Range, class... _Args>
         __result.reserve(static_cast<range_size_t<_Container>>(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 a983745fd636e8..26c7af6d88479b 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<C4>(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<int, CtrChoice::DefaultCtrAndInsert, InserterChoice::PushBack, true>;
+
+  std::ranges::contiguous_range auto r = adl_only_range{};
+  auto v                               = r | std::ranges::to<Cont>();
+  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;
 }



More information about the libcxx-commits mailing list