On Wed, May 27, 2026 at 3:04 PM Patrick Palka <[email protected]> wrote:
> This implements the changes in sections 5, 6 and 8 of P3567R2. The
> changes in section 4 and 7 were already effectively implemented.
>
> libstdc++-v3/ChangeLog:
>
> * include/std/flat_map (_Flat_map_impl): Remove
> is_nothrow_swappable_v assertions.
> (_Flat_map_impl::_Flat_map_impl): Explicitly default copy ctor.
> Define move ctor with corrected exception handling as per
> P3567R2.
> (_Flat_map_impl::operator=): Likewise.
> (_Flat_map_impl::insert_range): Define new __sorted_t overload
> as per P3567R2.
> (_Flat_map_impl::swap): Make conditionally noexcept as per
> P3567R2.
> * include/std/flat_set (_Flat_set_impl): Remove
> is_nothrow_swappable_v assertion.
> (_Flat_set_impl::_Flat_set_impl): Explicitly default copy ctor.
> Define move ctor with correct invariant preserving behavior as
> per P3567R2.
> (_Flat_set_impl::operator=): Likewise.
> (_Flat_set_impl::swap): Make conditionally noexcept as per
> P3567R2.
> * libstdc++-v3/testsuite/23_containers/flat_map/1.cc
> (test11, test12): New tests.
> * libstdc++-v3/testsuite/23_containers/flat_multimap/1.cc
> (test11, test12): New tests.
> * libstdc++-v3/testsuite/23_containers/flat_multiset/1.cc
> (test10): New test.
> * libstdc++-v3/testsuite/23_containers/flat_set/1.cc
> (test10): New test.
> ---
>
Some comments follow, mostly regarding lacking guards inside a swap,
and unrelated suggestion for optimization of insert_range for map:
for pair-like we can extract via get direclty.
> libstdc++-v3/include/std/flat_map | 49 ++++++++--
> libstdc++-v3/include/std/flat_set | 40 ++++++++-
> .../testsuite/23_containers/flat_map/1.cc | 90 +++++++++++++++++++
> .../23_containers/flat_multimap/1.cc | 90 +++++++++++++++++++
> .../23_containers/flat_multiset/1.cc | 67 ++++++++++++++
> .../testsuite/23_containers/flat_set/1.cc | 67 ++++++++++++++
> 6 files changed, 395 insertions(+), 8 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/flat_map
> b/libstdc++-v3/include/std/flat_map
> index a51c6625d54a..b42c2356474d 100644
> --- a/libstdc++-v3/include/std/flat_map
> +++ b/libstdc++-v3/include/std/flat_map
> @@ -74,9 +74,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> static_assert(is_same_v<_Key, typename _KeyContainer::value_type>);
> static_assert(is_same_v<_Tp, typename
> _MappedContainer::value_type>);
>
> - static_assert(is_nothrow_swappable_v<_KeyContainer>);
> - static_assert(is_nothrow_swappable_v<_MappedContainer>);
> -
> using _Derived = __conditional_t<_Multi,
> flat_multimap<_Key, _Tp, _Compare,
> _KeyContainer,
> _MappedContainer>,
> @@ -395,6 +392,39 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> : _Flat_map_impl(__s, __il.begin(), __il.end(), __comp, __a)
> { }
>
> + _Flat_map_impl(const _Flat_map_impl&) = default;
> + _Flat_map_impl& operator=(const _Flat_map_impl&) = default;
> +
> + _GLIBCXX26_CONSTEXPR
> + _Flat_map_impl(_Flat_map_impl&& __other)
> + noexcept(is_nothrow_move_constructible_v<containers>
> + && is_nothrow_move_constructible_v<key_compare>)
> + try
> + : _M_cont(std::move(__other._M_cont)),
> _M_comp(std::move(__other._M_comp))
> + {
> + __other.clear();
> + }
> + catch (...)
> + {
> + __other.clear();
> + throw;
> + }
> +
> + _GLIBCXX26_CONSTEXPR
> + _Flat_map_impl&
> + operator=(_Flat_map_impl&& __other)
> + noexcept(is_nothrow_move_assignable_v<containers>
> + && is_nothrow_move_assignable_v<key_compare>)
> + {
> + auto __guard = _M_make_clear_guard();
> + auto __other_guard = _ClearGuard{__other._M_cont};
> + _M_cont = std::move(__other._M_cont);
> + _M_comp = std::move(__other._M_comp);
> + __guard._M_disable();
> + // __other_guard._M_disable is deliberately not called.
> + return *this;
> + }
> +
> _GLIBCXX26_CONSTEXPR
> _Derived&
> operator=(initializer_list<value_type> __il)
> @@ -644,6 +674,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> insert_range(_Rg&& __rg)
> { _M_insert(ranges::begin(__rg), ranges::end(__rg)); }
>
> + template<__detail::__container_compatible_range<value_type> _Rg>
> + _GLIBCXX26_CONSTEXPR
> + void
> + insert_range(__sorted_t, _Rg&& __rg)
> + { _M_insert(ranges::begin(__rg), ranges::end(__rg), true); }
>
While you are touching insert_range, we could check if range_reference is
__pair_like, and in this case move directly from the elements, instead of
creating
a temporary value_type. But this could be a separate commit.
> +
> _GLIBCXX26_CONSTEXPR
> void
> insert(initializer_list<value_type> __il)
> @@ -728,7 +764,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
> _GLIBCXX26_CONSTEXPR
> void
> - swap(_Derived& __y) noexcept
> + swap(_Derived& __y)
> + noexcept(is_nothrow_swappable_v<key_container_type>
> + && is_nothrow_swappable_v<mapped_container_type>
> + && is_nothrow_swappable_v<key_compare>)
> {
> ranges::swap(_M_comp, __y._M_comp);
> ranges::swap(_M_cont.keys, __y._M_cont.keys);
I think we break invariants here, if the keys are successfully swapped, but
Values are not. I think we may need to add two clear guards here,
similarly, as they are done for the assignment in this patch.
Also, I think I would keep the order the same, and move container first,
and then comparator.
> @@ -974,7 +1013,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> }
>
> friend _GLIBCXX26_CONSTEXPR void
> - swap(_Derived& __x, _Derived& __y) noexcept
> + swap(_Derived& __x, _Derived& __y) noexcept(noexcept(__x.swap(__y)))
> { return __x.swap(__y); }
>
> template<typename _Predicate>
> diff --git a/libstdc++-v3/include/std/flat_set
> b/libstdc++-v3/include/std/flat_set
> index 286290896ae0..ff4a9fb2f10f 100644
> --- a/libstdc++-v3/include/std/flat_set
> +++ b/libstdc++-v3/include/std/flat_set
> @@ -71,7 +71,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> class _Flat_set_impl
> {
> static_assert(is_same_v<_Key, typename _KeyContainer::value_type>);
> - static_assert(is_nothrow_swappable_v<_KeyContainer>);
>
> using _Derived = __conditional_t<_Multi,
> flat_multiset<_Key, _Compare,
> _KeyContainer>,
> @@ -324,6 +323,39 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> : _Flat_set_impl(__s, __il.begin(), __il.end(), __comp, __a)
> { }
>
> + _Flat_set_impl(const _Flat_set_impl&) = default;
> + _Flat_set_impl& operator=(const _Flat_set_impl&) = default;
> +
> + _GLIBCXX26_CONSTEXPR
> + _Flat_set_impl(_Flat_set_impl&& __other)
> + noexcept(is_nothrow_move_constructible_v<container_type>
> + && is_nothrow_move_constructible_v<key_compare>)
> + try
> + : _M_cont(std::move(__other._M_cont)),
> _M_comp(std::move(__other._M_comp))
> + {
> + __other.clear();
> + }
> + catch (...)
> + {
> + __other.clear();
> + throw;
> + }
> +
> + _GLIBCXX26_CONSTEXPR
> + _Flat_set_impl&
> + operator=(_Flat_set_impl&& __other)
> + noexcept(is_nothrow_move_assignable_v<container_type>
> + && is_nothrow_move_assignable_v<key_compare>)
> + {
> + auto __guard = _M_make_clear_guard();
> + auto __other_guard = _ClearGuard{__other._M_cont};
> + _M_cont = std::move(__other._M_cont);
> + _M_comp = std::move(__other._M_comp);
> + __guard._M_disable();
> + // __other_guard._M_disable is deliberately not called.
> + return *this;
> + }
> +
> _GLIBCXX26_CONSTEXPR
> _Derived&
> operator=(initializer_list<value_type> __il)
> @@ -628,7 +660,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
> _GLIBCXX26_CONSTEXPR
> void
> - swap(_Derived& __x) noexcept
> + swap(_Derived& __x)
> + noexcept(is_nothrow_swappable_v<container_type>
> + && is_nothrow_swappable_v<key_compare>)
> {
> using std::swap;
> swap(_M_cont, __x._M_cont);
> @@ -830,7 +864,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> }
>
> friend _GLIBCXX26_CONSTEXPR void
> - swap(_Derived& __x, _Derived& __y) noexcept
> + swap(_Derived& __x, _Derived& __y) noexcept(noexcept(__x.swap(__y)))
> { return __x.swap(__y); }
>
> template<typename _Predicate>
> diff --git a/libstdc++-v3/testsuite/23_containers/flat_map/1.cc
> b/libstdc++-v3/testsuite/23_containers/flat_map/1.cc
> index 1cb088f511eb..538a4bcc7766 100644
> --- a/libstdc++-v3/testsuite/23_containers/flat_map/1.cc
> +++ b/libstdc++-v3/testsuite/23_containers/flat_map/1.cc
> @@ -299,6 +299,93 @@ test10()
> VERIFY (k == "world");
> }
>
> +void
> +test11()
> +{
> + static bool throw_on_move = false;
> +
> + struct vector : std::vector<int>
> + {
> + vector() = default;
> + vector(const vector&) = default;
> + vector& operator=(const vector&) = default;
> +
> + vector(vector&& other) : std::vector<int>(std::move(other))
> + {
> + if (throw_on_move)
> + throw std::runtime_error("move ctor");
> + }
> +
> + vector&
> + operator=(vector&& other)
> + {
> + static_cast<std::vector<int>&>(*this) = std::move(other);
> + if (throw_on_move)
> + throw std::runtime_error("move assign");
> + return *this;
> + }
> + };
> +
> + auto check = []<typename KC, typename MC>() {
> + using flat_map = std::flat_map<int, int, std::less<int>, KC, MC>;
> +
> + // Verify invariant preservation upon throwing move construction.
> + flat_map source;
> + source.insert({1, 100});
> + source.insert({2, 200});
> + try
> + {
> + throw_on_move = true;
> + flat_map target(std::move(source));
> + VERIFY( false );
> + }
> + catch (const std::runtime_error&)
> + {
> + throw_on_move = false;
> + auto extracted = std::move(source).extract();
>
Could you check if container itself is empty, before extracting keys and
values?
> + VERIFY( extracted.keys.empty() );
> + VERIFY( extracted.values.empty() );
> + }
> +
> + // Verify invariant preservation upon throwing move assignment.
> + source.clear();
> + source.insert({1, 100});
> + source.insert({2, 200});
> + flat_map target;
> + target.insert({3, 300});
> + target.insert({4, 400});
> + try
> + {
> + throw_on_move = true;
> + target = std::move(source);
> + VERIFY( false );
> + }
> + catch (const std::runtime_error&)
> + {
> + throw_on_move = false;
> + auto extracted = std::move(source).extract();
> + VERIFY( extracted.keys.empty() );
> + VERIFY( extracted.values.empty() );
> + extracted = std::move(target).extract();
> + VERIFY( extracted.keys.empty() );
> + VERIFY( extracted.values.empty() );
> + }
> + };
> + check.template operator()<vector, std::vector<int>>();
> + check.template operator()<std::vector<int>, vector>();
> +}
> +
> +constexpr
> +void
> +test12()
> +{
> + std::flat_map<int, int> m;
> + std::pair<int, int> s[] = {{1, 100}, {2, 200}};
> + m.insert_range(std::sorted_unique, s);
> + VERIFY( std::ranges::equal(m.keys(), (int[]){1, 2}) );
> + VERIFY( std::ranges::equal(m.values(), (int[]){100, 200}) );
> +}
> +
> void
> test()
> {
> @@ -315,6 +402,8 @@ test()
> test08();
> test09();
> test10();
> + test11();
> + test12();
> }
>
> constexpr
> @@ -333,6 +422,7 @@ test_constexpr()
> #if __cpp_lib_constexpr_string >= 201907L
> test10();
> #endif
> + test12();
> return true;
> }
>
> diff --git a/libstdc++-v3/testsuite/23_containers/flat_multimap/1.cc
> b/libstdc++-v3/testsuite/23_containers/flat_multimap/1.cc
> index 00644c14061a..14fc78318da7 100644
> --- a/libstdc++-v3/testsuite/23_containers/flat_multimap/1.cc
> +++ b/libstdc++-v3/testsuite/23_containers/flat_multimap/1.cc
> @@ -252,6 +252,93 @@ test09()
> using value_type = std::pair<int, int>;
> }
>
> +void
> +test11()
> +{
> + static bool throw_on_move = false;
> +
> + struct vector : std::vector<int>
> + {
> + vector() = default;
> + vector(const vector&) = default;
> + vector& operator=(const vector&) = default;
> +
> + vector(vector&& other) : std::vector<int>(std::move(other))
> + {
> + if (throw_on_move)
> + throw std::runtime_error("move ctor");
> + }
> +
> + vector&
> + operator=(vector&& other)
> + {
> + static_cast<std::vector<int>&>(*this) = std::move(other);
> + if (throw_on_move)
> + throw std::runtime_error("move assign");
> + return *this;
> + }
> + };
> +
> + auto check = []<typename KC, typename MC>() {
> + using flat_multimap = std::flat_multimap<int, int, std::less<int>,
> KC, MC>;
> +
> + // Verify invariant preservation upon throwing move construction.
> + flat_multimap source;
> + source.insert({1, 100});
> + source.insert({2, 200});
> + try
> + {
> + throw_on_move = true;
> + flat_multimap target(std::move(source));
> + VERIFY( false );
> + }
> + catch (const std::runtime_error&)
> + {
> + throw_on_move = false;
> + auto extracted = std::move(source).extract();
> + VERIFY( extracted.keys.empty() );
> + VERIFY( extracted.values.empty() );
> + }
> +
> + // Verify invariant preservation upon throwing move assignment.
> + source.clear();
> + source.insert({1, 100});
> + source.insert({2, 200});
> + flat_multimap target;
> + target.insert({3, 300});
> + target.insert({4, 400});
> + try
> + {
> + throw_on_move = true;
> + target = std::move(source);
> + VERIFY( false );
> + }
> + catch (const std::runtime_error&)
> + {
> + throw_on_move = false;
> + auto extracted = std::move(source).extract();
> + VERIFY( extracted.keys.empty() );
> + VERIFY( extracted.values.empty() );
> + extracted = std::move(target).extract();
> + VERIFY( extracted.keys.empty() );
> + VERIFY( extracted.values.empty() );
> + }
> + };
> + check.template operator()<vector, std::vector<int>>();
> + check.template operator()<std::vector<int>, vector>();
> +}
> +
> +constexpr
> +void
> +test12()
> +{
> + std::flat_multimap<int, int> m;
> + std::pair<int, int> s[] = {{1, 100}, {2, 200}};
> + m.insert_range(std::sorted_equivalent, s);
> + VERIFY( std::ranges::equal(m.keys(), (int[]){1, 2}) );
> + VERIFY( std::ranges::equal(m.values(), (int[]){100, 200}) );
> +}
> +
> void
> test()
> {
> @@ -266,6 +353,8 @@ test()
> test06();
> test07();
> test09();
> + test11();
> + test12();
> }
>
> constexpr
> @@ -280,6 +369,7 @@ test_constexpr()
> test06();
> test07();
> test09();
> + test12();
> return true;
> }
>
> diff --git a/libstdc++-v3/testsuite/23_containers/flat_multiset/1.cc
> b/libstdc++-v3/testsuite/23_containers/flat_multiset/1.cc
> index be330fefdd05..ee8b16b4ccd4 100644
> --- a/libstdc++-v3/testsuite/23_containers/flat_multiset/1.cc
> +++ b/libstdc++-v3/testsuite/23_containers/flat_multiset/1.cc
> @@ -265,6 +265,72 @@ test09()
> VERIFY( std::ranges::equal(s, (int[]){2,2,4}) );
> }
>
> +void
> +test10()
> +{
> + static bool throw_on_move = false;
> +
> + struct vector : std::vector<int>
> + {
> + vector() = default;
> + vector(const vector&) = default;
> + vector& operator=(const vector&) = default;
> +
> + vector(vector&& other) : std::vector<int>(std::move(other))
> + {
> + if (throw_on_move)
> + throw std::runtime_error("move ctor");
> + }
> +
> + vector&
> + operator=(vector&& other)
> + {
> + static_cast<std::vector<int>&>(*this) = std::move(other);
> + if (throw_on_move)
> + throw std::runtime_error("move assign");
> + return *this;
> + }
> + };
> +
> + auto check = [] {
> + using flat_multiset = std::flat_multiset<int, std::less<int>, vector>;
> +
> + // Verify invariant preservation upon throwing move construction.
> + flat_multiset source = {1, 2};
> + try
> + {
> + throw_on_move = true;
> + flat_multiset target(std::move(source));
> + VERIFY( false );
> + }
> + catch (const std::runtime_error&)
> + {
> + throw_on_move = false;
> + auto extracted = std::move(source).extract();
> + VERIFY( extracted.empty() );
> + }
> +
> + // Verify invariant preservation upon throwing move assignment.
> + source = {1, 2};
> + flat_multiset target = {3, 4};
> + try
> + {
> + throw_on_move = true;
> + target = std::move(source);
> + VERIFY( false );
> + }
> + catch (const std::runtime_error&)
> + {
> + throw_on_move = false;
> + auto extracted = std::move(source).extract();
> + VERIFY( extracted.empty() );
> + extracted = std::move(target).extract();
> + VERIFY( extracted.empty() );
> + }
> + };
> + check();
> +}
> +
> void
> test()
> {
> @@ -278,6 +344,7 @@ test()
> test07();
> test08();
> test09();
> + test10();
> }
>
> constexpr
> diff --git a/libstdc++-v3/testsuite/23_containers/flat_set/1.cc
> b/libstdc++-v3/testsuite/23_containers/flat_set/1.cc
> index a14d03476e18..2f08d25a714e 100644
> --- a/libstdc++-v3/testsuite/23_containers/flat_set/1.cc
> +++ b/libstdc++-v3/testsuite/23_containers/flat_set/1.cc
> @@ -277,6 +277,72 @@ test09()
> VERIFY( std::ranges::equal(s, (int[]){2,4}) );
> }
>
> +void
> +test10()
> +{
> + static bool throw_on_move = false;
> +
> + struct vector : std::vector<int>
> + {
> + vector() = default;
> + vector(const vector&) = default;
> + vector& operator=(const vector&) = default;
> +
> + vector(vector&& other) : std::vector<int>(std::move(other))
> + {
> + if (throw_on_move)
> + throw std::runtime_error("move ctor");
> + }
> +
> + vector&
> + operator=(vector&& other)
> + {
> + static_cast<std::vector<int>&>(*this) = std::move(other);
> + if (throw_on_move)
> + throw std::runtime_error("move assign");
> + return *this;
> + }
> + };
> +
> + auto check = [] {
> + using flat_set = std::flat_set<int, std::less<int>, vector>;
> +
> + // Verify invariant preservation upon throwing move construction.
> + flat_set source = {1, 2};
> + try
> + {
> + throw_on_move = true;
> + flat_set target(std::move(source));
> + VERIFY( false );
> + }
> + catch (const std::runtime_error&)
> + {
> + throw_on_move = false;
> + auto extracted = std::move(source).extract();
> + VERIFY( extracted.empty() );
> + }
> +
> + // Verify invariant preservation upon throwing move assignment.
> + source = {1, 2};
> + flat_set target = {3, 4};
> + try
> + {
> + throw_on_move = true;
> + target = std::move(source);
> + VERIFY( false );
> + }
> + catch (const std::runtime_error&)
> + {
> + throw_on_move = false;
> + auto extracted = std::move(source).extract();
> + VERIFY( extracted.empty() );
> + extracted = std::move(target).extract();
> + VERIFY( extracted.empty() );
> + }
> + };
> + check();
> +}
> +
> void
> test()
> {
> @@ -290,6 +356,7 @@ test()
> test07();
> test08();
> test09();
> + test10();
> }
>
> constexpr
> --
> 2.54.0.rc1.54.g60f07c4f5c
>
>
