pw_containers: Add methods to erase by item

Currently the `erase` methods of lists and maps accept iterators, and in
the latter case, keys. This CL adds an overload that also takes an item.
This improves the usability of these containers.

Change-Id: Id13f0a7c0c89daa5e361c14c3bc891f7612c5a4d
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/243257
Reviewed-by: Wyatt Hepler <[email protected]>
Commit-Queue: Aaron Green <[email protected]>
Lint: Lint 🤖 <[email protected]>
Docs-Not-Needed: Aaron Green <[email protected]>

pw_containers/intrusive_list_test.cc

@@ -698,7 +698,7 @@ TEST(IntrusiveListTest, Insert_SameItemAfterEnd) {
 }
 #endif  // TESTING_CHECK_FAILURES_IS_SUPPORTED
 
-TEST(IntrusiveListTest, Erase_FirstItem) {
+TEST(IntrusiveListTest, Erase_First_ByIterator) {
   std::array<Item, 3> items{{{0}, {1}, {2}}};
   List list(items.begin(), items.end());
 
@@ -708,7 +708,33 @@ TEST(IntrusiveListTest, Erase_FirstItem) {
   list.clear();
 }
 
-TEST(IntrusiveListTest, Erase_LastItem) {
+TEST(IntrusiveListTest, Erase_First_ByItem) {
+  std::array<Item, 3> items{{{0}, {1}, {2}}};
+  List list(items.begin(), items.end());
+
+  auto erased = list.erase(items[0]);
+  auto iter = list.begin();
+  EXPECT_EQ(erased, iter);
+  EXPECT_EQ(&items[1], &(*iter++));
+  EXPECT_EQ(&items[2], &(*iter++));
+  EXPECT_EQ(list.end(), iter);
+  list.clear();
+}
+
+TEST(IntrusiveListTest, Erase_Middle_ByItem) {
+  std::array<Item, 3> items{{{0}, {1}, {2}}};
+  List list(items.begin(), items.end());
+
+  auto erased = list.erase(items[1]);
+  auto iter = list.begin();
+  EXPECT_EQ(&items[0], &(*iter++));
+  EXPECT_EQ(erased, iter);
+  EXPECT_EQ(&items[2], &(*iter++));
+  EXPECT_EQ(list.end(), iter);
+  list.clear();
+}
+
+TEST(IntrusiveListTest, Erase_Last_ByIterator) {
   std::array<Item, 3> items{{{0}, {1}, {2}}};
   List list(items.begin(), items.end());
 
@@ -725,6 +751,19 @@ TEST(IntrusiveListTest, Erase_LastItem) {
   list.clear();
 }
 
+TEST(IntrusiveListTest, Erase_Last_ByItem) {
+  std::array<Item, 3> items{{{0}, {1}, {2}}};
+  List list(items.begin(), items.end());
+
+  auto erased = list.erase(items[2]);
+  auto iter = list.begin();
+  EXPECT_EQ(&items[0], &(*iter++));
+  EXPECT_EQ(&items[1], &(*iter++));
+  EXPECT_EQ(erased, iter);
+  EXPECT_EQ(list.end(), iter);
+  list.clear();
+}
+
 TEST(IntrusiveListTest, Erase_AllItems) {
   std::array<Item, 3> items{{{0}, {1}, {2}}};
   List list(items.begin(), items.end());

pw_containers/intrusive_map_test.cc

@@ -485,7 +485,20 @@ TEST_F(IntrusiveMapTest, Insert_DerivedItems_CompilationFails) {
   derived_from_compatible_item_type.clear();
 }
 
-TEST_F(IntrusiveMapTest, Erase_OneItem) {
+TEST_F(IntrusiveMapTest, Erase_One_ByItem) {
+  for (size_t i = 0; i < kNumItems; ++i) {
+    EXPECT_EQ(map_.size(), kNumItems);
+    auto iter = map_.erase(items_[i]);
+    if (iter != map_.end()) {
+      EXPECT_GT(iter->key(), items_[i].key());
+    }
+    EXPECT_EQ(map_.size(), kNumItems - 1);
+    EXPECT_EQ(map_.find(items_[i].key()), map_.end());
+    map_.insert(items_[i]);
+  }
+}
+
+TEST_F(IntrusiveMapTest, Erase_One_ByKey) {
   for (size_t i = 0; i < kNumItems; ++i) {
     EXPECT_EQ(map_.size(), kNumItems);
     EXPECT_EQ(map_.erase(items_[i].key()), 1U);

pw_containers/intrusive_multimap_test.cc

@@ -511,6 +511,18 @@ TEST_F(IntrusiveMultiMapTest, Insert_DerivedItems_CompilationFails) {
   derived_from_compatible_item_type.clear();
 }
 
+TEST_F(IntrusiveMultiMapTest, Erase_One_ByItem) {
+  for (size_t i = 0; i < kNumItems; ++i) {
+    EXPECT_EQ(multimap_.size(), kNumItems);
+    auto iter = multimap_.erase(items_[i]);
+    if (iter != multimap_.end()) {
+      EXPECT_GE(iter->key(), items_[i].key());
+    }
+    EXPECT_EQ(multimap_.size(), kNumItems - 1);
+    multimap_.insert(items_[i]);
+  }
+}
+
 TEST_F(IntrusiveMultiMapTest, Erase_Two_ByKey) {
   constexpr size_t kHalf = kNumItems / 2;
   for (size_t i = 0; i < kHalf; ++i) {

pw_containers/public/pw_containers/intrusive_list.h

@@ -72,6 +72,9 @@ namespace containers::future {
 ///   - std::list<T>::erase
 ///   - std::list<T>::splice
 ///
+/// - An additional overload of `erase` is provided that takes a direct
+///   reference to an item.
+///
 /// - C++23 methods are not (yet) supported.
 ///
 /// @tparam   T           Type of intrusive items stored in the list.
@@ -214,7 +217,11 @@ class IntrusiveList {
     return insert(pos, items.begin(), items.end());
   }
 
-  /// Removes the item following pos from the list. The item is not destructed.
+  /// Removes the given `item` from the list. The item is not destructed.
+  iterator erase(T& item) { return erase(iterator(&item)); }
+
+  /// Removes the item following `pos` from the list. The item is not
+  /// destructed.
   iterator erase(iterator pos) {
     return iterator(list_.erase_after((--pos).item_));
   }

pw_containers/public/pw_containers/intrusive_map.h

@@ -102,6 +102,9 @@ class IntrusiveMapItemWithKey : public containers::internal::IntrusiveMapItem<
 ///   - std::map<T>::insert
 ///   - std::map<T>::erase
 ///
+/// - An additional overload of `erase` is provided that takes a direct
+///   reference to an item.
+///
 /// - C++23 methods are not (yet) supported.
 ///
 /// @tparam   Key         Type to sort items on
@@ -264,6 +267,8 @@ class IntrusiveMap {
   /// the removed item..
   ///
   /// The items themselves are not destructed.
+  iterator erase(T& item) { return iterator(tree_.erase(item)); }
+
   iterator erase(iterator pos) { return iterator(tree_.erase(*pos)); }
 
   iterator erase(iterator first, iterator last) {

pw_containers/public/pw_containers/intrusive_multimap.h

@@ -50,6 +50,9 @@ namespace pw {
 ///   - std::multimap<T>::insert
 ///   - std::multimap<T>::erase
 ///
+/// - An additional overload of `erase` is provided that takes a direct
+///   reference to an item.
+///
 /// - C++23 methods are not (yet) supported.
 ///
 /// @tparam   Key         Type to sort items on
@@ -186,6 +189,8 @@ class IntrusiveMultiMap {
   /// after the removed item.
   ///
   /// The items themselves are not destructed.
+  iterator erase(T& item) { return iterator(tree_.erase(item)); }
+
   iterator erase(iterator pos) { return iterator(tree_.erase(*pos)); }
 
   iterator erase(iterator first, iterator last) {