On Wed, 27 May 2026, Tomasz Kaminski wrote:

> 
> 
> 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.

Will look into that as a follow-up

>  
>       +
>              _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. 

The current order matches the Effects: clause of the standard:
https://eel.is/c++draft/flat.map#modifiers-34

Would you know why it's written this way?  Perhaps it's better
to swap the comparator first so that if that throws we don't
have to clear keys and values.

>  
>       @@ -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? 

Will do.

>       +       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
> 
> 
> 

Reply via email to