4016. container-insertable checks do not match what container-inserter does

Section: 25.5.7 [range.utility.conv] Status: WP Submitter: Jonathan Wakely Opened: 2023-11-24 Last modified: 2024-04-02

Priority: Not Prioritized

View all issues with WP status.

Discussion:

The exposition-only helper container-inserter uses either std::back_inserter or std::inserter. Both std::back_insert_iterator and std::insert_iterator require C::value_type to be a valid type, and we do not check for that in container-insertable. The insert iterators can also incur a conversion to construct a C::value_type which then gets moved into the container. Using emplace instead of insert would avoid that temporary object. It's also possible (although arguably not worth caring about) that range_value_t<C> is not the same type as C::value_type, and that conversion to C::value_type could be ill-formed (we only check that conversion from range_reference_t<R> to range_value_t<C> is well-formed).

It seems preferable to remove the use of insert iterators, so that we don't need to check their requirements at all.

[2023-1-26; Rename exposition-only concept and function after reflector discussion.]

[2024-03-11; Reflector poll]

Set status to Tentatively Ready after six votes in favour during reflector poll.

[Tokyo 2024-03-23; Status changed: Voting → WP.]

Proposed resolution:

This wording is relative to N4964.

1. Modify 25.5.7.1 [range.utility.conv.general] as indicated:

   -4- Let container-insertableappendable be defined as follows:

   template<class Container, class Ref>
   constexpr bool container-insertableappendable =         // exposition only
     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)); });
   };
   

   -5- Let container-inserterappend be defined as follows:

   template<class Container, class Ref>
   constexpr auto container-inserterappend(Container& c) {     // exposition only
     if constexpr (requires { c.push_back(declval<Ref>()); })
       return back_inserter(c);
     else
       return inserter(c, c.end());
     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
         c.insert(c.end(), std::forward<Ref>(ref));
     };
   };
   

2. Modify 25.5.7.2 [range.utility.conv.to] as indicated:

   (2.1.4) Otherwise, if

   • — constructible_from<C, Args...> is true, and
   • — container-insertableappendable<C, range_reference_t<R>> is true:

   
   C c(std::forward<Args>(args)...);
   if constexpr (sized_range<R> && reservable-container<C>)
   c.reserve(static_cast<range_size_t<C>>(ranges::size(r)));
   ranges::copyfor_each(r, container-inserterappend<range_reference_t<R>>(c));