[libc++][ranges] LWG4016: container-insertable checks do not match what container-inserter does (#113103)

This patch implements LWG4016: container-insertable checks do not match
what container-inserter does.

Author: Xiaoyang Liu

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|","",""

libcxx/include/__ranges/to.h

@@ -10,15 +10,12 @@
 #ifndef _LIBCPP___RANGES_TO_H
 #define _LIBCPP___RANGES_TO_H
 
-#include <__algorithm/ranges_copy.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,21 +51,14 @@ 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());
-  }
-}
-
 // Note: making this a concept allows short-circuiting the second condition.
 template <class _Container, class _Range>
 concept __try_non_recursive_conversion =
@@ -113,14 +103,25 @@ 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));
-
+      for (auto&& __ref : __range) {
+        using _Ref = decltype(__ref);
+        if constexpr (requires { __result.emplace_back(declval<_Ref>()); }) {
+          __result.emplace_back(std::forward<_Ref>(__ref));
+        } else if constexpr (requires { __result.push_back(declval<_Ref>()); }) {
+          __result.push_back(std::forward<_Ref>(__ref));
+        } else if constexpr (requires { __result.emplace(__result.end(), declval<_Ref>()); }) {
+          __result.emplace(__result.end(), std::forward<_Ref>(__ref));
+        } else {
+          static_assert(requires { __result.insert(__result.end(), declval<_Ref>()); });
+          __result.insert(__result.end(), std::forward<_Ref>(__ref));
+        }
+      }
       return __result;
 
     } else {

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;
   }
 

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>();
   }
 }