[libcxx-commits] [libcxx] [libc++][ranges] LWG4016: container-insertable checks do not match what container-inserter does (PR #113103)
Xiaoyang Liu via libcxx-commits
libcxx-commits at lists.llvm.org
Sun Oct 20 12:41:07 PDT 2024
https://github.com/xiaoyang-sde created https://github.com/llvm/llvm-project/pull/113103
## 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
>From 97d7316b5118c4e703ba3c43a35676b325df961a Mon Sep 17 00:00:00 2001
From: Xiaoyang Liu <siujoeng.lau at gmail.com>
Date: Sun, 20 Oct 2024 15:36:47 -0400
Subject: [PATCH 1/2] [libc++][ranges] LWG4016: container-insertable checks do
not match what container-inserter does
---
libcxx/docs/Status/Cxx2cIssues.csv | 2 +-
libcxx/include/__ranges/to.h | 29 +++--
.../range.utility.conv/container.h | 34 +++++-
.../range.utility.conv/to.pass.cpp | 105 ++++++++----------
4 files changed, 91 insertions(+), 79 deletions(-)
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..0bb18cc330cd6b 100644
--- a/libcxx/include/__ranges/to.h
+++ b/libcxx/include/__ranges/to.h
@@ -10,7 +10,7 @@
#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>
@@ -54,19 +54,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 +120,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>();
}
}
>From 482b830a99bee541213728dd2f03c6e95c16200d Mon Sep 17 00:00:00 2001
From: Xiaoyang Liu <siujoeng.lau at gmail.com>
Date: Sun, 20 Oct 2024 15:40:31 -0400
Subject: [PATCH 2/2] [libc++][ranges] LWG4016: container-insertable checks do
not match what container-inserter does
---
libcxx/include/__ranges/to.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/libcxx/include/__ranges/to.h b/libcxx/include/__ranges/to.h
index 0bb18cc330cd6b..5651db2fc3af4b 100644
--- a/libcxx/include/__ranges/to.h
+++ b/libcxx/include/__ranges/to.h
@@ -17,8 +17,6 @@
#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>
More information about the libcxx-commits
mailing list