[libcxx-commits] [libcxx] [libc++][ranges] LWG4016: container-insertable checks do not match what container-inserter does (PR #113103)
via libcxx-commits
libcxx-commits at lists.llvm.org
Sun Oct 20 12:41:39 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx
Author: Xiaoyang Liu (xiaoyang-sde)
<details>
<summary>Changes</summary>
## Introduction
This patch implements LWG4016: container-insertable checks do not match what container-inserter does.
## Reference
- [[range.utility.conv.to]](https://eel.is/c++draft/range.utility.conv.to)
- [LWG4016](https://cplusplus.github.io/LWG/issue4016)
Closes #<!-- -->105322
---
Full diff: https://github.com/llvm/llvm-project/pull/113103.diff
4 Files Affected:
- (modified) libcxx/docs/Status/Cxx2cIssues.csv (+1-1)
- (modified) libcxx/include/__ranges/to.h (+18-13)
- (modified) libcxx/test/std/ranges/range.utility/range.utility.conv/container.h (+28-6)
- (modified) libcxx/test/std/ranges/range.utility/range.utility.conv/to.pass.cpp (+44-61)
``````````diff
diff --git a/libcxx/docs/Status/Cxx2cIssues.csv b/libcxx/docs/Status/Cxx2cIssues.csv
index a62c4992020a0f..6766c69a170a06 100644
--- a/libcxx/docs/Status/Cxx2cIssues.csv
+++ b/libcxx/docs/Status/Cxx2cIssues.csv
@@ -48,7 +48,7 @@
"`LWG4011 <https://wg21.link/LWG4011>`__","``""Effects: Equivalent to return""`` in ``[span.elem]``","2024-03 (Tokyo)","|Nothing To Do|","",""
"`LWG4012 <https://wg21.link/LWG4012>`__","``common_view::begin/end`` are missing the ``simple-view`` check","2024-03 (Tokyo)","","",""
"`LWG4013 <https://wg21.link/LWG4013>`__","``lazy_split_view::outer-iterator::value_type`` should not provide default constructor","2024-03 (Tokyo)","","",""
-"`LWG4016 <https://wg21.link/LWG4016>`__","container-insertable checks do not match what container-inserter does","2024-03 (Tokyo)","","",""
+"`LWG4016 <https://wg21.link/LWG4016>`__","container-insertable checks do not match what container-inserter does","2024-03 (Tokyo)","|Complete|","20.0",""
"`LWG4023 <https://wg21.link/LWG4023>`__","Preconditions of ``std::basic_streambuf::setg/setp``","2024-03 (Tokyo)","|Complete|","19.0",""
"`LWG4025 <https://wg21.link/LWG4025>`__","Move assignment operator of ``std::expected<cv void, E>`` should not be conditionally deleted","2024-03 (Tokyo)","|Complete|","20.0",""
"`LWG4030 <https://wg21.link/LWG4030>`__","Clarify whether arithmetic expressions in ``[numeric.sat.func]`` are mathematical or C++","2024-03 (Tokyo)","|Nothing To Do|","",""
diff --git a/libcxx/include/__ranges/to.h b/libcxx/include/__ranges/to.h
index 85fc580b53c9bc..5651db2fc3af4b 100644
--- a/libcxx/include/__ranges/to.h
+++ b/libcxx/include/__ranges/to.h
@@ -10,15 +10,13 @@
#ifndef _LIBCPP___RANGES_TO_H
#define _LIBCPP___RANGES_TO_H
-#include <__algorithm/ranges_copy.h>
+#include <__algorithm/ranges_for_each.h>
#include <__concepts/constructible.h>
#include <__concepts/convertible_to.h>
#include <__concepts/derived_from.h>
#include <__concepts/same_as.h>
#include <__config>
#include <__functional/bind_back.h>
-#include <__iterator/back_insert_iterator.h>
-#include <__iterator/insert_iterator.h>
#include <__iterator/iterator_traits.h>
#include <__ranges/access.h>
#include <__ranges/concepts.h>
@@ -54,19 +52,26 @@ constexpr bool __reservable_container =
};
template <class _Container, class _Ref>
-constexpr bool __container_insertable = requires(_Container& __c, _Ref&& __ref) {
+constexpr bool __container_appendable = requires(_Container& __c, _Ref&& __ref) {
requires(
+ requires { __c.emplace_back(std::forward<_Ref>(__ref)); } ||
requires { __c.push_back(std::forward<_Ref>(__ref)); } ||
+ requires { __c.emplace(__c.end(), std::forward<_Ref>(__ref)); } ||
requires { __c.insert(__c.end(), std::forward<_Ref>(__ref)); });
};
-template <class _Ref, class _Container>
-_LIBCPP_HIDE_FROM_ABI constexpr auto __container_inserter(_Container& __c) {
- if constexpr (requires { __c.push_back(std::declval<_Ref>()); }) {
- return std::back_inserter(__c);
- } else {
- return std::inserter(__c, __c.end());
- }
+template <class _Container>
+_LIBCPP_HIDE_FROM_ABI constexpr auto __container_append(_Container& __c) {
+ return [&__c]<class _Ref>(_Ref&& __ref) {
+ if constexpr (requires { __c.emplace_back(declval<_Ref>()); })
+ __c.emplace_back(std::forward<_Ref>(__ref));
+ else if constexpr (requires { __c.push_back(declval<_Ref>()); })
+ __c.push_back(std::forward<_Ref>(__ref));
+ else if constexpr (requires { __c.emplace(__c.end(), declval<_Ref>()); })
+ __c.emplace(__c.end(), std::forward<_Ref>(__ref));
+ else if constexpr (requires { __c.insert(__c.end(), declval<_Ref>()); })
+ __c.insert(__c.end(), std::forward<_Ref>(__ref));
+ };
}
// Note: making this a concept allows short-circuiting the second condition.
@@ -113,13 +118,13 @@ template <class _Container, input_range _Range, class... _Args>
// Case 4 -- default-construct (or construct from the extra arguments) and insert, reserving the size if possible.
else if constexpr (constructible_from<_Container, _Args...> &&
- __container_insertable<_Container, range_reference_t<_Range>>) {
+ __container_appendable<_Container, range_reference_t<_Range>>) {
_Container __result(std::forward<_Args>(__args)...);
if constexpr (sized_range<_Range> && __reservable_container<_Container>) {
__result.reserve(static_cast<range_size_t<_Container>>(ranges::size(__range)));
}
- ranges::copy(__range, ranges::__container_inserter<range_reference_t<_Range>>(__result));
+ ranges::for_each(__range, ranges::__container_append(__result));
return __result;
diff --git a/libcxx/test/std/ranges/range.utility/range.utility.conv/container.h b/libcxx/test/std/ranges/range.utility/range.utility.conv/container.h
index ca89e3757affc7..4fd52680add9b3 100644
--- a/libcxx/test/std/ranges/range.utility/range.utility.conv/container.h
+++ b/libcxx/test/std/ranges/range.utility/range.utility.conv/container.h
@@ -10,12 +10,11 @@
#define RANGES_RANGE_UTILITY_RANGE_UTILITY_CONV_CONTAINER_H
#include <algorithm>
-#include <concepts>
#include <cstddef>
enum class CtrChoice { Invalid, DefaultCtrAndInsert, BeginEndPair, FromRangeT, DirectCtr };
-enum class InserterChoice { Invalid, Insert, PushBack };
+enum class InserterChoice { Invalid, Insert, Emplace, PushBack, EmplaceBack };
// Allows checking that `ranges::to` correctly follows the order of priority of different constructors -- e.g., if
// 3 constructors are available, the `from_range_t` constructor is chosen in favor of the constructor taking two
@@ -96,27 +95,50 @@ struct Container {
constexpr ElementType* end() { return buffer_ + size_; }
constexpr std::size_t size() const { return size_; }
+ template <class T>
+ constexpr void emplace_back(T val)
+ requires(Inserter >= InserterChoice::EmplaceBack)
+ {
+ inserter_choice = InserterChoice::EmplaceBack;
+ __push_back_impl(val);
+ }
+
template <class T>
constexpr void push_back(T val)
requires(Inserter >= InserterChoice::PushBack)
{
inserter_choice = InserterChoice::PushBack;
- buffer_[size_] = val;
+ __push_back_impl(val);
+ }
+
+ template <class T>
+ constexpr void __push_back_impl(T val) {
+ buffer_[size_] = val;
++size_;
}
+ template <class T>
+ constexpr ElementType* emplace(ElementType* where, T val)
+ requires(Inserter >= InserterChoice::Emplace)
+ {
+ inserter_choice = InserterChoice::Emplace;
+ return __insert_impl(where, val);
+ }
+
template <class T>
constexpr ElementType* insert(ElementType* where, T val)
requires(Inserter >= InserterChoice::Insert)
{
- assert(size() + 1 <= Capacity);
-
inserter_choice = InserterChoice::Insert;
+ return __insert_impl(where, val);
+ }
+ template <class T>
+ constexpr ElementType* __insert_impl(ElementType* where, T val) {
+ assert(size() + 1 <= Capacity);
std::shift_right(where, end(), 1);
*where = val;
++size_;
-
return where;
}
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 7f816bb21a1978..a983745fd636e8 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
@@ -392,72 +392,55 @@ constexpr void test_ctr_choice_order() {
}
{ // Case 4 -- default-construct then insert elements.
- {
- using C = Container<int, CtrChoice::DefaultCtrAndInsert, InserterChoice::Insert, /*CanReserve=*/false>;
- std::same_as<C> decltype(auto) result = std::ranges::to<C>(in);
-
- assert(result.ctr_choice == CtrChoice::DefaultCtrAndInsert);
- assert(result.inserter_choice == InserterChoice::Insert);
- assert(std::ranges::equal(result, in));
- assert(!result.called_reserve);
- assert((in | std::ranges::to<C>()) == result);
- auto closure = std::ranges::to<C>();
- assert((in | closure) == result);
- }
-
- {
- using C = Container<int, CtrChoice::DefaultCtrAndInsert, InserterChoice::Insert, /*CanReserve=*/true>;
- std::same_as<C> decltype(auto) result = std::ranges::to<C>(in);
-
- assert(result.ctr_choice == CtrChoice::DefaultCtrAndInsert);
- assert(result.inserter_choice == InserterChoice::Insert);
- assert(std::ranges::equal(result, in));
- assert(result.called_reserve);
- assert((in | std::ranges::to<C>()) == result);
- auto closure = std::ranges::to<C>();
- assert((in | closure) == result);
- }
-
- {
- using C = Container<int, CtrChoice::DefaultCtrAndInsert, InserterChoice::PushBack, /*CanReserve=*/false>;
- std::same_as<C> decltype(auto) result = std::ranges::to<C>(in);
+ auto case_4 = [in, arg1, arg2]<auto InserterChoice, bool CanReserve>() {
+ using C = Container<int, CtrChoice::DefaultCtrAndInsert, InserterChoice, CanReserve>;
+ {
+ [[maybe_unused]] std::same_as<C> decltype(auto) result = std::ranges::to<C>(in);
+
+ assert(result.ctr_choice == CtrChoice::DefaultCtrAndInsert);
+ assert(result.inserter_choice == InserterChoice);
+ assert(std::ranges::equal(result, in));
+
+ if constexpr (CanReserve) {
+ assert(result.called_reserve);
+ } else {
+ assert(!result.called_reserve);
+ }
- assert(result.ctr_choice == CtrChoice::DefaultCtrAndInsert);
- assert(result.inserter_choice == InserterChoice::PushBack);
- assert(std::ranges::equal(result, in));
- assert(!result.called_reserve);
- assert((in | std::ranges::to<C>()) == result);
- auto closure = std::ranges::to<C>();
- assert((in | closure) == result);
- }
+ assert((in | std::ranges::to<C>()) == result);
+ [[maybe_unused]] auto closure = std::ranges::to<C>();
+ assert((in | closure) == result);
+ }
- {
- using C = Container<int, CtrChoice::DefaultCtrAndInsert, InserterChoice::PushBack, /*CanReserve=*/true>;
- std::same_as<C> decltype(auto) result = std::ranges::to<C>(in);
+ { // Extra arguments
+ [[maybe_unused]] std::same_as<C> decltype(auto) result = std::ranges::to<C>(in, arg1, arg2);
- assert(result.ctr_choice == CtrChoice::DefaultCtrAndInsert);
- assert(result.inserter_choice == InserterChoice::PushBack);
- assert(std::ranges::equal(result, in));
- assert(result.called_reserve);
- assert((in | std::ranges::to<C>()) == result);
- auto closure = std::ranges::to<C>();
- assert((in | closure) == result);
- }
+ assert(result.ctr_choice == CtrChoice::DefaultCtrAndInsert);
+ assert(result.inserter_choice == InserterChoice);
+ assert(std::ranges::equal(result, in));
+ assert(result.extra_arg1 == arg1);
+ assert(result.extra_arg2 == arg2);
- { // Extra arguments.
- using C = Container<int, CtrChoice::DefaultCtrAndInsert, InserterChoice::Insert, /*CanReserve=*/false>;
- std::same_as<C> decltype(auto) result = std::ranges::to<C>(in, arg1, arg2);
+ if constexpr (CanReserve) {
+ assert(result.called_reserve);
+ } else {
+ assert(!result.called_reserve);
+ }
- assert(result.ctr_choice == CtrChoice::DefaultCtrAndInsert);
- assert(result.inserter_choice == InserterChoice::Insert);
- assert(std::ranges::equal(result, in));
- assert(!result.called_reserve);
- assert(result.extra_arg1 == arg1);
- assert(result.extra_arg2 == arg2);
- assert((in | std::ranges::to<C>(arg1, arg2)) == result);
- auto closure = std::ranges::to<C>(arg1, arg2);
- assert((in | closure) == result);
- }
+ assert((in | std::ranges::to<C>(arg1, arg2)) == result);
+ [[maybe_unused]] auto closure = std::ranges::to<C>(arg1, arg2);
+ assert((in | closure) == result);
+ }
+ };
+
+ case_4.operator()<InserterChoice::Insert, false>();
+ case_4.operator()<InserterChoice::Insert, true>();
+ case_4.operator()<InserterChoice::Emplace, false>();
+ case_4.operator()<InserterChoice::Emplace, true>();
+ case_4.operator()<InserterChoice::PushBack, false>();
+ case_4.operator()<InserterChoice::PushBack, true>();
+ case_4.operator()<InserterChoice::EmplaceBack, false>();
+ case_4.operator()<InserterChoice::EmplaceBack, true>();
}
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/113103
More information about the libcxx-commits
mailing list