[base] Return size_t from Erase and EraseIf

This change modifies base's implementation of C++20's uniform container
erasure API to reflect the latest changes from the committee meeting in
Prague. In particular, these functions now return the number of erased
elements, rather than having no return value. Differences from the
standard [1]:

- For simplicity, base::Erase and base::EraseIf return size_t, rather
  than typename Container::size_type.
- Since std::forward_list::remove_if only returns size_type starting
  from C++20, the respective overloads need to perform two extra O(n)
  operations to calculate the number of removed elements.

[1] https://wg21.link/p1115

Bug: 697235
Change-Id: I30ad6ce08f3a37eddd1483e4fdc4f1cd8d155d2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2072038Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#744996}

base/containers/circular_deque.h

@@ -1096,15 +1096,19 @@ class circular_deque {
 
 // Implementations of base::Erase[If] (see base/stl_util.h).
 template <class T, class Value>
-void Erase(circular_deque<T>& container, const Value& value) {
-  container.erase(std::remove(container.begin(), container.end(), value),
-                  container.end());
+size_t Erase(circular_deque<T>& container, const Value& value) {
+  auto it = std::remove(container.begin(), container.end(), value);
+  size_t removed = std::distance(it, container.end());
+  container.erase(it, container.end());
+  return removed;
 }
 
 template <class T, class Predicate>
-void EraseIf(circular_deque<T>& container, Predicate pred) {
-  container.erase(std::remove_if(container.begin(), container.end(), pred),
-                  container.end());
+size_t EraseIf(circular_deque<T>& container, Predicate pred) {
+  auto it = std::remove_if(container.begin(), container.end(), pred);
+  size_t removed = std::distance(it, container.end());
+  container.erase(it, container.end());
+  return removed;
 }
 
 }  // namespace base

base/containers/flat_tree.h

@@ -934,11 +934,14 @@ template <class Key,
           class GetKeyFromValue,
           class KeyCompare,
           typename Predicate>
-void EraseIf(base::internal::flat_tree<Key, Value, GetKeyFromValue, KeyCompare>&
-                 container,
-             Predicate pred) {
-  container.erase(std::remove_if(container.begin(), container.end(), pred),
-                  container.end());
+size_t EraseIf(
+    base::internal::flat_tree<Key, Value, GetKeyFromValue, KeyCompare>&
+        container,
+    Predicate pred) {
+  auto it = std::remove_if(container.begin(), container.end(), pred);
+  size_t removed = std::distance(it, container.end());
+  container.erase(it, container.end());
+  return removed;
 }
 
 }  // namespace base

base/containers/flat_tree_unittest.cc

@@ -1281,15 +1281,15 @@ TEST(FlatTree, Comparison) {
 
 TEST(FlatSet, EraseIf) {
   IntTree x;
-  EraseIf(x, [](int) { return false; });
+  EXPECT_EQ(0u, EraseIf(x, [](int) { return false; }));
   EXPECT_THAT(x, ElementsAre());
 
   x = {1, 2, 3};
-  EraseIf(x, [](int elem) { return !(elem & 1); });
+  EXPECT_EQ(1u, EraseIf(x, [](int elem) { return !(elem & 1); }));
   EXPECT_THAT(x, ElementsAre(1, 3));
 
   x = {1, 2, 3, 4};
-  EraseIf(x, [](int elem) { return elem & 1; });
+  EXPECT_EQ(2u, EraseIf(x, [](int elem) { return elem & 1; }));
   EXPECT_THAT(x, ElementsAre(2, 4));
 }
 

base/stl_util.h

@@ -31,15 +31,18 @@ namespace base {
 
 namespace internal {
 
-// Calls erase on iterators of matching elements.
+// Calls erase on iterators of matching elements and returns the number of
+// removed elements.
 template <typename Container, typename Predicate>
-void IterateAndEraseIf(Container& container, Predicate pred) {
-  for (auto it = container.begin(); it != container.end();) {
+size_t IterateAndEraseIf(Container& container, Predicate pred) {
+  size_t old_size = container.size();
+  for (auto it = container.begin(), last = container.end(); it != last;) {
     if (pred(*it))
       it = container.erase(it);
     else
       ++it;
   }
+  return old_size - container.size();
 }
 
 template <typename Iter>
@@ -497,8 +500,9 @@ bool STLIncludes(const Arg1& a1, const Arg2& a2) {
                        a2.begin(), a2.end());
 }
 
-// Erase/EraseIf are based on library fundamentals ts v2 erase/erase_if
-// http://en.cppreference.com/w/cpp/experimental/lib_extensions_2
+// Erase/EraseIf are based on C++20's uniform container erasure API:
+// - https://eel.is/c++draft/libraryindex#:erase
+// - https://eel.is/c++draft/libraryindex#:erase_if
 // They provide a generic way to erase elements from a container.
 // The functions here implement these for the standard containers until those
 // functions are available in the C++ standard.
@@ -508,89 +512,109 @@ bool STLIncludes(const Arg1& a1, const Arg2& a2) {
 // have it either.
 
 template <typename CharT, typename Traits, typename Allocator, typename Value>
-void Erase(std::basic_string<CharT, Traits, Allocator>& container,
-           const Value& value) {
-  container.erase(std::remove(container.begin(), container.end(), value),
-                  container.end());
+size_t Erase(std::basic_string<CharT, Traits, Allocator>& container,
+             const Value& value) {
+  auto it = std::remove(container.begin(), container.end(), value);
+  size_t removed = std::distance(it, container.end());
+  container.erase(it, container.end());
+  return removed;
 }
 
 template <typename CharT, typename Traits, typename Allocator, class Predicate>
-void EraseIf(std::basic_string<CharT, Traits, Allocator>& container,
-             Predicate pred) {
-  container.erase(std::remove_if(container.begin(), container.end(), pred),
-                  container.end());
+size_t EraseIf(std::basic_string<CharT, Traits, Allocator>& container,
+               Predicate pred) {
+  auto it = std::remove_if(container.begin(), container.end(), pred);
+  size_t removed = std::distance(it, container.end());
+  container.erase(it, container.end());
+  return removed;
 }
 
 template <class T, class Allocator, class Value>
-void Erase(std::deque<T, Allocator>& container, const Value& value) {
-  container.erase(std::remove(container.begin(), container.end(), value),
-                  container.end());
+size_t Erase(std::deque<T, Allocator>& container, const Value& value) {
+  auto it = std::remove(container.begin(), container.end(), value);
+  size_t removed = std::distance(it, container.end());
+  container.erase(it, container.end());
+  return removed;
 }
 
 template <class T, class Allocator, class Predicate>
-void EraseIf(std::deque<T, Allocator>& container, Predicate pred) {
-  container.erase(std::remove_if(container.begin(), container.end(), pred),
-                  container.end());
+size_t EraseIf(std::deque<T, Allocator>& container, Predicate pred) {
+  auto it = std::remove_if(container.begin(), container.end(), pred);
+  size_t removed = std::distance(it, container.end());
+  container.erase(it, container.end());
+  return removed;
 }
 
 template <class T, class Allocator, class Value>
-void Erase(std::vector<T, Allocator>& container, const Value& value) {
-  container.erase(std::remove(container.begin(), container.end(), value),
-                  container.end());
+size_t Erase(std::vector<T, Allocator>& container, const Value& value) {
+  auto it = std::remove(container.begin(), container.end(), value);
+  size_t removed = std::distance(it, container.end());
+  container.erase(it, container.end());
+  return removed;
 }
 
 template <class T, class Allocator, class Predicate>
-void EraseIf(std::vector<T, Allocator>& container, Predicate pred) {
-  container.erase(std::remove_if(container.begin(), container.end(), pred),
-                  container.end());
+size_t EraseIf(std::vector<T, Allocator>& container, Predicate pred) {
+  auto it = std::remove_if(container.begin(), container.end(), pred);
+  size_t removed = std::distance(it, container.end());
+  container.erase(it, container.end());
+  return removed;
 }
 
 template <class T, class Allocator, class Value>
-void Erase(std::forward_list<T, Allocator>& container, const Value& value) {
+size_t Erase(std::forward_list<T, Allocator>& container, const Value& value) {
   // Unlike std::forward_list::remove, this function template accepts
   // heterogeneous types and does not force a conversion to the container's
   // value type before invoking the == operator.
-  container.remove_if([&](const T& cur) { return cur == value; });
+  return EraseIf(container, [&](const T& cur) { return cur == value; });
 }
 
 template <class T, class Allocator, class Predicate>
-void EraseIf(std::forward_list<T, Allocator>& container, Predicate pred) {
+size_t EraseIf(std::forward_list<T, Allocator>& container, Predicate pred) {
+  // Note: std::forward_list does not have a size() API, thus we need to use the
+  // O(n) std::distance work-around. However, given that EraseIf is O(n)
+  // already, this should not make a big difference.
+  size_t old_size = std::distance(container.begin(), container.end());
   container.remove_if(pred);
+  return old_size - std::distance(container.begin(), container.end());
 }
 
 template <class T, class Allocator, class Value>
-void Erase(std::list<T, Allocator>& container, const Value& value) {
+size_t Erase(std::list<T, Allocator>& container, const Value& value) {
   // Unlike std::list::remove, this function template accepts heterogeneous
   // types and does not force a conversion to the container's value type before
   // invoking the == operator.
-  container.remove_if([&](const T& cur) { return cur == value; });
+  return EraseIf(container, [&](const T& cur) { return cur == value; });
 }
 
 template <class T, class Allocator, class Predicate>
-void EraseIf(std::list<T, Allocator>& container, Predicate pred) {
+size_t EraseIf(std::list<T, Allocator>& container, Predicate pred) {
+  size_t old_size = container.size();
   container.remove_if(pred);
+  return old_size - container.size();
 }
 
 template <class Key, class T, class Compare, class Allocator, class Predicate>
-void EraseIf(std::map<Key, T, Compare, Allocator>& container, Predicate pred) {
-  internal::IterateAndEraseIf(container, pred);
+size_t EraseIf(std::map<Key, T, Compare, Allocator>& container,
+               Predicate pred) {
+  return internal::IterateAndEraseIf(container, pred);
 }
 
 template <class Key, class T, class Compare, class Allocator, class Predicate>
-void EraseIf(std::multimap<Key, T, Compare, Allocator>& container,
-             Predicate pred) {
-  internal::IterateAndEraseIf(container, pred);
+size_t EraseIf(std::multimap<Key, T, Compare, Allocator>& container,
+               Predicate pred) {
+  return internal::IterateAndEraseIf(container, pred);
 }
 
 template <class Key, class Compare, class Allocator, class Predicate>
-void EraseIf(std::set<Key, Compare, Allocator>& container, Predicate pred) {
-  internal::IterateAndEraseIf(container, pred);
+size_t EraseIf(std::set<Key, Compare, Allocator>& container, Predicate pred) {
+  return internal::IterateAndEraseIf(container, pred);
 }
 
 template <class Key, class Compare, class Allocator, class Predicate>
-void EraseIf(std::multiset<Key, Compare, Allocator>& container,
-             Predicate pred) {
-  internal::IterateAndEraseIf(container, pred);
+size_t EraseIf(std::multiset<Key, Compare, Allocator>& container,
+               Predicate pred) {
+  return internal::IterateAndEraseIf(container, pred);
 }
 
 template <class Key,
@@ -599,9 +623,9 @@ template <class Key,
           class KeyEqual,
           class Allocator,
           class Predicate>
-void EraseIf(std::unordered_map<Key, T, Hash, KeyEqual, Allocator>& container,
-             Predicate pred) {
-  internal::IterateAndEraseIf(container, pred);
+size_t EraseIf(std::unordered_map<Key, T, Hash, KeyEqual, Allocator>& container,
+               Predicate pred) {
+  return internal::IterateAndEraseIf(container, pred);
 }
 
 template <class Key,
@@ -610,10 +634,10 @@ template <class Key,
           class KeyEqual,
           class Allocator,
           class Predicate>
-void EraseIf(
+size_t EraseIf(
     std::unordered_multimap<Key, T, Hash, KeyEqual, Allocator>& container,
     Predicate pred) {
-  internal::IterateAndEraseIf(container, pred);
+  return internal::IterateAndEraseIf(container, pred);
 }
 
 template <class Key,
@@ -621,9 +645,9 @@ template <class Key,
           class KeyEqual,
           class Allocator,
           class Predicate>
-void EraseIf(std::unordered_set<Key, Hash, KeyEqual, Allocator>& container,
-             Predicate pred) {
-  internal::IterateAndEraseIf(container, pred);
+size_t EraseIf(std::unordered_set<Key, Hash, KeyEqual, Allocator>& container,
+               Predicate pred) {
+  return internal::IterateAndEraseIf(container, pred);
 }
 
 template <class Key,
@@ -631,9 +655,10 @@ template <class Key,
           class KeyEqual,
           class Allocator,
           class Predicate>
-void EraseIf(std::unordered_multiset<Key, Hash, KeyEqual, Allocator>& container,
-             Predicate pred) {
-  internal::IterateAndEraseIf(container, pred);
+size_t EraseIf(
+    std::unordered_multiset<Key, Hash, KeyEqual, Allocator>& container,
+    Predicate pred) {
+  return internal::IterateAndEraseIf(container, pred);
 }
 
 // A helper class to be used as the predicate with |EraseIf| to implement

base/stl_util_unittest.cc

@@ -51,13 +51,25 @@ class ComparableValue {
   int value_;
 };
 
+template <typename Container>
+size_t GetSize(const Container& c) {
+  return c.size();
+}
+
+template <typename T>
+size_t GetSize(const std::forward_list<T>& l) {
+  return std::distance(l.begin(), l.end());
+}
+
 template <typename Container>
 void RunEraseTest() {
   const std::pair<Container, Container> test_data[] = {
       {Container(), Container()}, {{1, 2, 3}, {1, 3}}, {{1, 2, 3, 2}, {1, 3}}};
 
   for (auto test_case : test_data) {
-    base::Erase(test_case.first, 2);
+    size_t expected_erased =
+        GetSize(test_case.first) - GetSize(test_case.second);
+    EXPECT_EQ(expected_erased, base::Erase(test_case.first, 2));
     EXPECT_EQ(test_case.second, test_case.first);
   }
 }
@@ -76,16 +88,21 @@ void RunEraseIfTest() {
   };
 
   for (auto test_case : test_data) {
-    base::EraseIf(test_case.input, [](const std::pair<int, int>& elem) {
-      return !(elem.first & 1);
-    });
+    size_t expected_erased =
+        GetSize(test_case.input) - GetSize(test_case.erase_even);
+    EXPECT_EQ(expected_erased,
+              base::EraseIf(test_case.input, [](const auto& elem) {
+                return !(elem.first & 1);
+              }));
     EXPECT_EQ(test_case.erase_even, test_case.input);
   }
 
   for (auto test_case : test_data) {
-    base::EraseIf(test_case.input, [](const std::pair<int, int>& elem) {
-      return elem.first & 1;
-    });
+    size_t expected_erased =
+        GetSize(test_case.input) - GetSize(test_case.erase_odd);
+    EXPECT_EQ(expected_erased,
+              base::EraseIf(test_case.input,
+                            [](const auto& elem) { return elem.first & 1; }));
     EXPECT_EQ(test_case.erase_odd, test_case.input);
   }
 }
@@ -604,24 +621,22 @@ TEST(Erase, List) {
 
 TEST(Erase, Map) {
   RunEraseIfTest<std::map<int, int>>();
-  RunEraseIfTest<std::map<int, int, std::greater<int>>>();
+  RunEraseIfTest<std::map<int, int, std::greater<>>>();
 }
 
 TEST(Erase, Multimap) {
   RunEraseIfTest<std::multimap<int, int>>();
-  RunEraseIfTest<std::multimap<int, int, std::greater<int>>>();
+  RunEraseIfTest<std::multimap<int, int, std::greater<>>>();
 }
 
 TEST(Erase, Set) {
   RunEraseIfTest<std::set<std::pair<int, int>>>();
-  RunEraseIfTest<
-      std::set<std::pair<int, int>, std::greater<std::pair<int, int>>>>();
+  RunEraseIfTest<std::set<std::pair<int, int>, std::greater<>>>();
 }
 
 TEST(Erase, Multiset) {
   RunEraseIfTest<std::multiset<std::pair<int, int>>>();
-  RunEraseIfTest<
-      std::multiset<std::pair<int, int>, std::greater<std::pair<int, int>>>>();
+  RunEraseIfTest<std::multiset<std::pair<int, int>, std::greater<>>>();
 }
 
 TEST(Erase, UnorderedMap) {
@@ -647,7 +662,7 @@ TEST(Erase, IsNotIn) {
   std::vector<int> lhs = {0, 2, 2, 4, 4, 4, 6, 8, 10};
   std::vector<int> rhs = {1, 2, 2, 4, 5, 6, 7};
   std::vector<int> expected = {2, 2, 4, 6};
-  EraseIf(lhs, IsNotIn<std::vector<int>>(rhs));
+  EXPECT_EQ(5u, EraseIf(lhs, IsNotIn<std::vector<int>>(rhs)));
   EXPECT_EQ(expected, lhs);
 }
 

base/values.h

@@ -229,9 +229,7 @@ class BASE_EXPORT Value {
   template <typename Predicate>
   size_t EraseListValueIf(Predicate pred) {
     CHECK(is_list());
-    const size_t old_size = list_.size();
-    base::EraseIf(list_, pred);
-    return old_size - list_.size();
+    return base::EraseIf(list_, pred);
   }
 
   // Erases all Values from the list.