review

Author: huixie90

libcxx/docs/ReleaseNotes/21.rst

@@ -43,8 +43,8 @@ Implemented Papers
 - P1361R2: Integration of chrono with text formatting (`Github <https://github.com/llvm/llvm-project/issues/100014>`__)
 - P2255R2: A type trait to detect reference binding to temporary (implemented the type traits only) (`Github <https://github.com/llvm/llvm-project/issues/105180>`__)
 - P2562R1: ``constexpr`` Stable Sorting (`Github <https://github.com/llvm/llvm-project/issues/105360>`__)
-- P1222R4: A Standard ``flat_set`` is partially implemented and ``flat_set`` is provided (`Github <https://github.com/llvm/llvm-project/issues/105193>`__)
 - P0472R3: Put std::monostate in <utility> (`Github <https://github.com/llvm/llvm-project/issues/127874>`__)
+- P1222R4: A Standard ``flat_set`` (`Github <https://github.com/llvm/llvm-project/issues/105193>`__)
 
 Improvements and New Features
 -----------------------------

libcxx/include/__flat_set/flat_multiset.h

@@ -41,6 +41,7 @@
 #include <__iterator/concepts.h>
 #include <__iterator/distance.h>
 #include <__iterator/iterator_traits.h>
+#include <__iterator/prev.h>
 #include <__iterator/ranges_iterator_traits.h>
 #include <__iterator/reverse_iterator.h>
 #include <__memory/allocator_traits.h>
@@ -442,7 +443,7 @@ class flat_multiset {
     return iterator(__key_iter);
   }
 
-  // iterator and const_iterator are the same type
+  // The following overload is the same as the iterator overload
   // iterator erase(const_iterator __position);
 
   _LIBCPP_HIDE_FROM_ABI size_type erase(const key_type& __x) {
@@ -473,7 +474,7 @@ class flat_multiset {
     // warning: The spec has unconditional noexcept, which means that
     // if any of the following functions throw an exception,
     // std::terminate will be called
-    // This is discussed in P2767, which hasn't been voted on yet.
+    // This is discussed in P3567, which hasn't been voted on yet.
     ranges::swap(__compare_, __y.__compare_);
     ranges::swap(__keys_, __y.__keys_);
   }
@@ -597,15 +598,13 @@ class flat_multiset {
     auto __on_failure    = std::__make_exception_guard([&]() noexcept { clear() /* noexcept */; });
     size_type __old_size = size();
     __flat_set_utils::__append(*this, std::forward<_Args>(__args)...);
-    if (size() != __old_size) {
-      if constexpr (!_WasSorted) {
-        ranges::sort(__keys_.begin() + __old_size, __keys_.end(), __compare_);
-      } else {
-        _LIBCPP_ASSERT_SEMANTIC_REQUIREMENT(
-            ranges::is_sorted(__keys_ | ranges::views::drop(__old_size)), "Key container is not sorted");
-      }
-      ranges::inplace_merge(__keys_.begin(), __keys_.begin() + __old_size, __keys_.end(), __compare_);
+    if constexpr (!_WasSorted) {
+      ranges::sort(__keys_.begin() + __old_size, __keys_.end(), __compare_);
+    } else {
+      _LIBCPP_ASSERT_SEMANTIC_REQUIREMENT(
+          ranges::is_sorted(__keys_ | ranges::views::drop(__old_size)), "Key container is not sorted");
     }
+    ranges::inplace_merge(__keys_.begin(), __keys_.begin() + __old_size, __keys_.end(), __compare_);
     __on_failure.__complete();
   }
 
@@ -621,7 +620,7 @@ class flat_multiset {
     auto __next_smaller = __hint != cend() && __compare_(*__hint, __key);
 
     if (!__prev_larger && !__next_smaller) [[likely]] {
-      // hint correct, just use exact hint iterators
+      // hint correct, just use exact hint iterator
     } else if (__prev_larger && !__next_smaller) {
       // the hint position is more to the right than the key should have been.
       // we want to emplace the element to a position as right as possible

libcxx/include/__flat_set/utils.h

@@ -49,8 +49,6 @@ struct __flat_set_utils {
     return typename decay_t<_Set>::iterator(std::move(__key_it));
   }
 
-  // TODO: We could optimize this, see
-  // https://github.com/llvm/llvm-project/issues/108624
   template <class _Set, class _InputIterator>
   _LIBCPP_HIDE_FROM_ABI static void __append(_Set& __set, _InputIterator __first, _InputIterator __last) {
     __set.__keys_.insert(__set.__keys_.end(), std::move(__first), std::move(__last));

libcxx/test/std/containers/container.adaptors/flat.multiset/flat.multiset.cons/copy_alloc.pass.cpp

@@ -12,6 +12,7 @@
 
 // flat_multiset(const flat_multiset&, const allocator_type&);
 
+#include <algorithm>
 #include <cassert>
 #include <deque>
 #include <flat_set>

libcxx/test/std/containers/container.adaptors/flat.multiset/flat.multiset.cons/initializer_list.pass.cpp

@@ -16,6 +16,7 @@
 // template<class Alloc>
 //    flat_multiset(initializer_list<value_type> il, const key_compare& comp, const Alloc& a);
 
+#include <algorithm>
 #include <cassert>
 #include <deque>
 #include <flat_set>

libcxx/test/std/containers/container.adaptors/flat.multiset/helpers.h

@@ -6,8 +6,8 @@
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef SUPPORT_flat_multiset_HELPERS_H
-#define SUPPORT_flat_multiset_HELPERS_H
+#ifndef SUPPORT_FLAT_MULTISET_HELPERS_H
+#define SUPPORT_FLAT_MULTISET_HELPERS_H
 
 #include <algorithm>
 #include <cassert>
@@ -298,4 +298,4 @@ class Moveable {
   bool moved() const { return int_ == -1; }
 };
 
-#endif // SUPPORT_flat_multiset_HELPERS_H
+#endif // SUPPORT_FLAT_MULTISET_HELPERS_H