On Mon, 24 Mar 2025, Tomasz Kamiński wrote:
> These patch add check to verify if common range iterators satisfies
> Cpp17LegacyIterator requirements (__detail::__cpp17_input_iterator),
> before invoking overloads of insert that accepts two iterators.
> As such overloads existed before c++20 iterators were introduced,
> they commonly assume existence of iterator_traits<..>::iterator_category,
> and passing a cpp20-only iterators, leads to hard errors.
>
> In case if user-defined container wants to support more efficient
> insertion in such cases, it should provided insert_range method,
> as in the case of standard containers.
>
> PR libstdc++/119415
>
> libstdc++-v3/ChangeLog:
>
> * include/std/flat_set (flat_multiset::insert_range,
> flat_set::insert_range): Add __detail::__cpp17_input_iterator check.
There's a chance this ChangeLog entry might not be accepted by the
pre-commit hook. It might need to be written as
* include/std/flat_set (flat_multiset::insert_range)
(flat_set::insert_range): Add __detail::__cpp17_input_iterator check.
as per the ChangeLog style guide[1] which says:
Break long lists of function names by closing continued lines with ‘)’,
rather than ‘,’, and opening the continuation with ‘(’.
OTOH, what the patch is lexically changing is _Flat_set_impl::insert_range,
so maybe the ChangeLog entry should just mention that?
[1]:
https://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html#Style-of-Change-Logs;
> * testsuite/23_containers/flat_multiset/1.cc: New tests
> * testsuite/23_containers/flat_set/1.cc: New tests
> ---
> Testing on x86_64-linux. OK for trunk?
>
> libstdc++-v3/include/std/flat_set | 3 +-
> .../23_containers/flat_multiset/1.cc | 18 ++++++++++++
> .../testsuite/23_containers/flat_set/1.cc | 28 +++++++++++++++++++
> 3 files changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/libstdc++-v3/include/std/flat_set
> b/libstdc++-v3/include/std/flat_set
> index 9240f2b9a2e..ea416e67503 100644
> --- a/libstdc++-v3/include/std/flat_set
> +++ b/libstdc++-v3/include/std/flat_set
> @@ -480,7 +480,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> typename container_type::iterator __it;
> if constexpr (requires { _M_cont.insert_range(_M_cont.end(), __rg); })
> __it = _M_cont.insert_range(_M_cont.end(), __rg);
> - else if constexpr (ranges::common_range<_Rg>)
> + else if constexpr (ranges::common_range<_Rg>
> + &&
> __detail::__cpp17_input_iterator<ranges::iterator_t<_Rg>>)
> __it = _M_cont.insert(_M_cont.end(), ranges::begin(__rg),
> ranges::end(__rg));
> else
> {
> diff --git a/libstdc++-v3/testsuite/23_containers/flat_multiset/1.cc
> b/libstdc++-v3/testsuite/23_containers/flat_multiset/1.cc
> index 910f5dca5be..0858be4a9fe 100644
> --- a/libstdc++-v3/testsuite/23_containers/flat_multiset/1.cc
> +++ b/libstdc++-v3/testsuite/23_containers/flat_multiset/1.cc
> @@ -143,6 +143,23 @@ test06()
> VERIFY( std::ranges::equal(s, (int[]){1, 2, 3, 4, 5}) );
> }
>
> +void test07()
> +{
> +#ifdef __SIZEOF_INT128__
> + // PR libstdc++/119415 - flat_foo::insert_range cannot handle common ranges
> + // on c++20 only iterators
> + auto r = std::views::iota(__int128(1), __int128(6));
> +
> + std::flat_multiset<int> s;
> + s.insert_range(r);
> + VERIFY( std::ranges::equal(s, (int[]){1, 2, 3, 4, 5}) );
> +
> + std::flat_multiset<int, std::less<int>, NoInsertRange<int>> s2;
> + s2.insert_range(r);
> + VERIFY( std::ranges::equal(s2, (int[]){1, 2, 3, 4, 5}) );
> +#endif
> +}
> +
> int
> main()
> {
> @@ -153,4 +170,5 @@ main()
> test04();
> test05();
> test06();
> + test07();
> }
> diff --git a/libstdc++-v3/testsuite/23_containers/flat_set/1.cc
> b/libstdc++-v3/testsuite/23_containers/flat_set/1.cc
> index f0eaac936bf..374be80bb9d 100644
> --- a/libstdc++-v3/testsuite/23_containers/flat_set/1.cc
> +++ b/libstdc++-v3/testsuite/23_containers/flat_set/1.cc
> @@ -8,6 +8,7 @@
>
> #include <deque>
> #include <ranges>
> +#include <spanstream>
> #include <vector>
> #include <testsuite_allocator.h>
> #include <testsuite_hooks.h>
> @@ -158,6 +159,32 @@ test06()
> VERIFY( std::ranges::equal(s, (int[]){1, 2, 3, 4, 5}) );
> }
>
> +template<typename T>
> +struct NoInsertRange : std::vector<T>
> +{
> + using std::vector<T>::vector;
> +
> + template<typename It, typename R>
> + void insert_range(typename std::vector<T>::const_iterator, R&&) = delete;
> +};
> +
> +void test07()
> +{
> +#ifdef __SIZEOF_INT128__
> + // PR libstdc++/119415 - flat_foo::insert_range cannot handle common ranges
> + // on c++20 only iterators
> + auto r = std::views::iota(__int128(1), __int128(6));
> +
> + std::flat_set<int> s;
> + s.insert_range(r);
> + VERIFY( std::ranges::equal(s, (int[]){1, 2, 3, 4, 5}) );
> +
> + std::flat_set<int, std::less<int>, NoInsertRange<int>> s2;
> + s2.insert_range(r);
> + VERIFY( std::ranges::equal(s2, (int[]){1, 2, 3, 4, 5}) );
> +#endif
> +}
> +
> int
> main()
> {
> @@ -168,4 +195,5 @@ main()
> test04();
> test05();
> test06();
> + test07();
> }
> --
> 2.48.1
>
>