review

Author: huixie90

libcxx/include/__flat_map/flat_map.h

@@ -1040,7 +1040,7 @@ class flat_map {
   friend __flat_map_utils;
 
   containers __containers_;
-  [[no_unique_address]] key_compare __compare_;
+  _LIBCPP_NO_UNIQUE_ADDRESS key_compare __compare_;
 
   struct __key_equiv {
     _LIBCPP_HIDE_FROM_ABI __key_equiv(key_compare __c) : __comp_(__c) {}

libcxx/include/__flat_map/flat_multimap.h

@@ -18,7 +18,7 @@
 #include <__algorithm/ranges_is_sorted.h>
 #include <__algorithm/ranges_lower_bound.h>
 #include <__algorithm/ranges_partition_point.h>
-#include <__algorithm/ranges_stable_sort.h>
+#include <__algorithm/ranges_sort.h>
 #include <__algorithm/ranges_unique.h>
 #include <__algorithm/ranges_upper_bound.h>
 #include <__algorithm/remove_if.h>
@@ -758,7 +758,7 @@ class flat_multimap {
 
   _LIBCPP_HIDE_FROM_ABI void __sort() {
     auto __zv = ranges::views::zip(__containers_.keys, __containers_.values);
-    ranges::stable_sort(__zv, __compare_, [](const auto& __p) -> decltype(auto) { return std::get<0>(__p); });
+    ranges::sort(__zv, __compare_, [](const auto& __p) -> decltype(auto) { return std::get<0>(__p); });
   }
 
   template <class _Self, class _KeyIter>
@@ -780,7 +780,7 @@ class flat_multimap {
         return __compare_(std::get<0>(__p1), std::get<0>(__p2));
       };
       if constexpr (!_WasSorted) {
-        ranges::stable_sort(__zv.begin() + __append_start_offset, __end, __compare_key);
+        ranges::sort(__zv.begin() + __append_start_offset, __end, __compare_key);
       } else {
         _LIBCPP_ASSERT_SEMANTIC_REQUIREMENT(
             __is_sorted(__containers_.keys | ranges::views::drop(__append_start_offset)),
@@ -850,7 +850,7 @@ class flat_multimap {
   friend __flat_map_utils;
 
   containers __containers_;
-  [[no_unique_address]] key_compare __compare_;
+  _LIBCPP_NO_UNIQUE_ADDRESS key_compare __compare_;
 
   struct __key_equiv {
     _LIBCPP_HIDE_FROM_ABI __key_equiv(key_compare __c) : __comp_(__c) {}

libcxx/include/__flat_map/utils.h

@@ -27,8 +27,13 @@ _LIBCPP_PUSH_MACROS
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-// It is easier to be-friend to have a struct with static functions
+// These utilities are defined in a class instead of a namespace so that this class can be befriended more easily.
 struct __flat_map_utils {
+  // Emplace a {key: value} into a flat_{multi}map, at the exact position that
+  // __it_key and __it_mapped point to, assuming that the key is not already present in the map.
+  // When an exception is thrown during the emplacement, the function will try its best to
+  // roll back the changes it made to the map. If it cannot roll back the changes, it will
+  // clear the map.
   template <class _Map, class _IterK, class _IterM, class _KeyArg, class... _MArgs>
   _LIBCPP_HIDE_FROM_ABI static typename _Map::iterator __emplace_exact_pos(
       _Map& __map, _IterK&& __it_key, _IterM&& __it_mapped, _KeyArg&& __key, _MArgs&&... __mapped_args) {
@@ -74,6 +79,7 @@ struct __flat_map_utils {
     return typename _Map::iterator(std::move(__key_it), std::move(__mapped_it));
   }
 
+  // TODO: We could optimize this, see
   // https://github.com/llvm/llvm-project/issues/108624
   template <class _Map, class _InputIterator, class _Sentinel>
   _LIBCPP_HIDE_FROM_ABI static typename _Map::size_type