pw_containers: Make item type deduction generic

This CL moves generic intrusive container and item functions to a standalone file, and renames them to avoid mentioning lists. This makes them available to be reused by other intrusive containers, such as the forthcoming intrusive maps. Affected functions include the item type deduction checks for and the assertions for containers and items before insertion or destruction. Change-Id: I5f6db3fe1a0fb5904349bbe021fa16cf62538721 Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/235105 Lint: Lint 🤖 <[email protected]> Reviewed-by: Wyatt Hepler <[email protected]> Commit-Queue: Aaron Green <[email protected]> Reviewed-by: Taylor Cramer <[email protected]> Docs-Not-Needed: Aaron Green <[email protected]>
google · Sep 18, 2024 · 7f4759a · 7f4759a
1 parent 25c8b84
commit 7f4759a
Show file tree

Hide file tree

Showing 9 changed files with 128 additions and 96 deletions.
diff --git a/pw_containers/BUILD.bazel b/pw_containers/BUILD.bazel
@@ -60,56 +60,51 @@ cc_library(
 )
 
 cc_library(
-    name = "intrusive_forward_list",
-    hdrs = [
-        "public/pw_containers/intrusive_forward_list.h",
-    ],
+    name = "intrusive",
+    srcs = ["intrusive.cc"],
+    hdrs = ["public/pw_containers/internal/intrusive.h"],
+    implementation_deps = ["//pw_assert"],
     includes = ["public"],
-    deps = [
-        ":config",
-        ":intrusive_list_common",
-    ],
 )
 
 cc_library(
-    name = "intrusive_list",
+    name = "intrusive_list_common",
     hdrs = [
-        "public/pw_containers/intrusive_list.h",
+        "public/pw_containers/internal/intrusive_list.h",
+        "public/pw_containers/internal/intrusive_list_item.h",
+        "public/pw_containers/internal/intrusive_list_iterator.h",
     ],
     includes = ["public"],
+    deps = [":intrusive"],
+)
+
+cc_library(
+    name = "intrusive_forward_list",
+    hdrs = ["public/pw_containers/intrusive_forward_list.h"],
+    includes = ["public"],
     deps = [
         ":config",
         ":intrusive_list_common",
-        ":legacy_intrusive_list",
     ],
 )
 
 cc_library(
-    name = "intrusive_list_common",
-    srcs = [
-        "intrusive_list.cc",
-    ],
-    hdrs = [
-        "public/pw_containers/internal/intrusive_list.h",
-        "public/pw_containers/internal/intrusive_list_item.h",
-        "public/pw_containers/internal/intrusive_list_iterator.h",
-    ],
+    name = "intrusive_list",
+    hdrs = ["public/pw_containers/intrusive_list.h"],
     includes = ["public"],
     deps = [
-        "//pw_assert",
+        ":config",
+        ":intrusive_list_common",
+        ":legacy_intrusive_list",
     ],
 )
 
 cc_library(
     name = "legacy_intrusive_list",
-    hdrs = [
-        "public/pw_containers/internal/legacy_intrusive_list.h",
-    ],
+    hdrs = ["public/pw_containers/internal/legacy_intrusive_list.h"],
     includes = ["public"],
     visibility = ["//visibility:private"],
-    deps = [
-        ":intrusive_forward_list",
-    ],
+    deps = [":intrusive_forward_list"],
 )
 
 cc_library(

diff --git a/pw_containers/BUILD.gn b/pw_containers/BUILD.gn
@@ -143,6 +143,23 @@ pw_source_set("wrapped_iterator") {
   public = [ "public/pw_containers/wrapped_iterator.h" ]
 }
 
+pw_source_set("intrusive") {
+  public_configs = [ ":public_include_path" ]
+  public = [ "public/pw_containers/internal/intrusive.h" ]
+  sources = [ "intrusive.cc" ]
+  deps = [ dir_pw_assert ]
+}
+
+pw_source_set("intrusive_list_common") {
+  public_configs = [ ":public_include_path" ]
+  public = [
+    "public/pw_containers/internal/intrusive_list.h",
+    "public/pw_containers/internal/intrusive_list_item.h",
+    "public/pw_containers/internal/intrusive_list_iterator.h",
+  ]
+  public_deps = [ ":intrusive" ]
+}
+
 pw_source_set("intrusive_forward_list") {
   public_configs = [ ":public_include_path" ]
   public = [ "public/pw_containers/intrusive_forward_list.h" ]
@@ -162,17 +179,6 @@ pw_source_set("intrusive_list") {
   ]
 }
 
-pw_source_set("intrusive_list_common") {
-  public_configs = [ ":public_include_path" ]
-  public = [
-    "public/pw_containers/internal/intrusive_list.h",
-    "public/pw_containers/internal/intrusive_list_item.h",
-    "public/pw_containers/internal/intrusive_list_iterator.h",
-  ]
-  sources = [ "intrusive_list.cc" ]
-  deps = [ dir_pw_assert ]
-}
-
 pw_source_set("legacy_intrusive_list") {
   public_configs = [ ":public_include_path" ]
   public = [ "public/pw_containers/internal/legacy_intrusive_list.h" ]

diff --git a/pw_containers/CMakeLists.txt b/pw_containers/CMakeLists.txt
@@ -146,6 +146,28 @@ pw_add_library(pw_containers.wrapped_iterator INTERFACE
     public
 )
 
+pw_add_library(pw_containers.intrusive STATIC
+  HEADERS
+    public/pw_containers/internal/intrusive.h
+  PUBLIC_INCLUDES
+    public
+  SOURCES
+    intrusive.cc
+  PRIVATE_DEPS
+    pw_assert
+)
+
+pw_add_library(pw_containers.intrusive_list_common INTERFACE
+  HEADERS
+    public/pw_containers/internal/intrusive_list.h
+    public/pw_containers/internal/intrusive_list_item.h
+    public/pw_containers/internal/intrusive_list_iterator.h
+  PUBLIC_INCLUDES
+    public
+  PUBLIC_DEPS
+    pw_containers.intrusive
+)
+
 pw_add_library(pw_containers.intrusive_forward_list INTERFACE
   HEADERS
     public/pw_containers/intrusive_forward_list.h
@@ -167,19 +189,6 @@ pw_add_library(pw_containers.intrusive_list INTERFACE
     pw_containers.legacy_intrusive_list
 )
 
-pw_add_library(pw_containers.intrusive_list_common STATIC
-  HEADERS
-    public/pw_containers/internal/intrusive_list.h
-    public/pw_containers/internal/intrusive_list_item.h
-    public/pw_containers/internal/intrusive_list_iterator.h
-  PUBLIC_INCLUDES
-    public
-  SOURCES
-    intrusive_list.cc
-  PRIVATE_DEPS
-    pw_assert
-)
-
 pw_add_library(pw_containers.legacy_intrusive_list INTERFACE
   HEADERS
     public/pw_containers/internal/legacy_intrusive_list.h

diff --git a/pw_containers/intrusive_list.cc → pw_containers/intrusive.cc b/pw_containers/intrusive_list.cc → pw_containers/intrusive.cc
@@ -12,20 +12,20 @@
 // License for the specific language governing permissions and limitations under
 // the License.
 
-#include "pw_containers/internal/intrusive_list.h"
+#include "pw_containers/internal/intrusive.h"
 
 #include "pw_assert/check.h"
-#include "pw_containers/internal/intrusive_list_item.h"
 
 namespace pw::containers::internal {
 
-void CheckUnlisted(bool unlisted) {
-  PW_CHECK(unlisted,
-           "Item is in an intrusive list and cannot be added or destroyed");
+void CheckIntrusiveContainerIsEmpty(bool empty) {
+  PW_CHECK(empty, "Intrusive container is not empty and cannot be destroyed");
 }
 
-void CheckEmpty(bool empty) {
-  PW_CHECK(empty, "List is not empty and cannot be destroyed");
+void CheckIntrusiveItemIsUncontained(bool uncontained) {
+  PW_CHECK(
+      uncontained,
+      "Item is in an intrusive container and cannot be added or destroyed");
 }
 
 }  // namespace pw::containers::internal
diff --git a/pw_containers/public/pw_containers/internal/intrusive.h b/pw_containers/public/pw_containers/internal/intrusive.h
@@ -0,0 +1,49 @@
+// Copyright 2024 The Pigweed Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+#pragma once
+
+#include <type_traits>
+
+namespace pw::containers::internal {
+
+/// Crashes with a diagnostic message that intrusive containers must be empty
+/// before destruction if the given `empty` parameter is not set.
+///
+/// This function is standalone to avoid using PW_CHECK in a header file.
+void CheckIntrusiveContainerIsEmpty(bool empty);
+
+/// Crashes with a diagnostic message that items must not be in an intrusive
+/// container before insertion into an intrusive container or before destruction
+/// if the given `uncontained` parameter is not set.
+///
+/// This function is standalone to avoid using PW_CHECK in a header file.
+void CheckIntrusiveItemIsUncontained(bool uncontained);
+
+// Gets the element type from an Item. This is used to check that an
+// IntrusiveList element class inherits from Item, either directly or through
+// another class.
+template <typename Item, typename T, bool kIsItem = std::is_base_of<Item, T>()>
+struct GetElementTypeFromItem {
+  using Type = void;
+};
+
+template <typename Item, typename T>
+struct GetElementTypeFromItem<Item, T, true> {
+  using Type = typename T::ElementType;
+};
+
+template <typename Item, typename T>
+using ElementTypeFromItem = typename GetElementTypeFromItem<Item, T>::Type;
+
+}  // namespace pw::containers::internal
diff --git a/pw_containers/public/pw_containers/internal/intrusive_list.h b/pw_containers/public/pw_containers/internal/intrusive_list.h
@@ -21,12 +21,6 @@
 
 namespace pw::containers::internal {
 
-/// Crashes with a diagnostic message that lists must be empty before
-/// destruction if the given `empty` parameter is not set.
-///
-/// This function is standalone to avoid using PW_CHECK in a header file.
-void CheckEmpty(bool empty);
-
 /// Generic intrusive list implementation.
 ///
 /// This implementation relies on the `Item` type to provide details of how to
@@ -51,7 +45,7 @@ class GenericIntrusiveList {
   GenericIntrusiveList(const GenericIntrusiveList&) = delete;
   GenericIntrusiveList& operator=(const GenericIntrusiveList&) = delete;
 
-  ~GenericIntrusiveList() { CheckEmpty(empty()); }
+  ~GenericIntrusiveList() { CheckIntrusiveContainerIsEmpty(empty()); }
 
   template <typename Iterator>
   void assign(Iterator first, Iterator last) {
@@ -107,7 +101,7 @@ class GenericIntrusiveList {
   ///
   /// @return The item that was added.
   static Item* insert_after(Item* prev, Item& item) {
-    CheckUnlisted(item.unlisted());
+    CheckIntrusiveItemIsUncontained(item.unlisted());
     item.next_ = prev->next_;
     item.set_previous(prev);
     prev->next_ = &item;
@@ -332,20 +326,4 @@ class GenericIntrusiveList {
   Item head_;
 };
 
-// Gets the element type from an Item. This is used to check that an
-// IntrusiveList element class inherits from Item, either directly or through
-// another class.
-template <typename Item, typename T, bool kIsItem = std::is_base_of<Item, T>()>
-struct GetListElementTypeFromItem {
-  using Type = void;
-};
-
-template <typename Item, typename T>
-struct GetListElementTypeFromItem<Item, T, true> {
-  using Type = typename T::PwIntrusiveListElementType;
-};
-
-template <typename Item, typename T>
-using ElementTypeFromItem = typename GetListElementTypeFromItem<Item, T>::Type;
-
 }  // namespace pw::containers::internal
diff --git a/pw_containers/public/pw_containers/internal/intrusive_list_item.h b/pw_containers/public/pw_containers/internal/intrusive_list_item.h
@@ -17,13 +17,6 @@
 
 namespace pw::containers::internal {
 
-/// Crashes with a diagnostic message that items must be unlisted before
-/// addition to a list or destruction if the given `unlisted` parameter is not
-/// set.
-///
-/// This function is standalone to avoid using PW_CHECK in a header file.
-void CheckUnlisted(bool unlisted);
-
 // Forward declaration for friending.
 template <typename>
 class GenericIntrusiveList;
@@ -41,7 +34,7 @@ class IntrusiveListItemBase {
   ///
   /// Subclasses MUST call `unlist` in their destructors, since unlisting may
   /// require calling derived class methods.
-  ~IntrusiveListItemBase() { CheckUnlisted(unlisted()); }
+  ~IntrusiveListItemBase() { CheckIntrusiveItemIsUncontained(unlisted()); }
 
   /// Items are not copyable.
   IntrusiveListItemBase(const IntrusiveListItemBase&) = delete;

diff --git a/pw_containers/public/pw_containers/intrusive_forward_list.h b/pw_containers/public/pw_containers/intrusive_forward_list.h
@@ -20,6 +20,7 @@
 #include <type_traits>
 
 #include "pw_containers/config.h"
+#include "pw_containers/internal/intrusive.h"
 #include "pw_containers/internal/intrusive_list.h"
 #include "pw_containers/internal/intrusive_list_item.h"
 #include "pw_containers/internal/intrusive_list_iterator.h"
@@ -118,12 +119,12 @@ class IntrusiveForwardList {
     constexpr explicit Item() = default;
 
    private:
-    // GetListElementTypeFromItem is used to find the element type from an item.
+    // GetElementTypeFromItem is used to find the element type from an item.
     // It is used to ensure list items inherit from the correct Item type.
     template <typename, typename, bool>
-    friend struct ::pw::containers::internal::GetListElementTypeFromItem;
+    friend struct ::pw::containers::internal::GetElementTypeFromItem;
 
-    using PwIntrusiveListElementType = T;
+    using ElementType = T;
   };
 
   using iterator =

diff --git a/pw_containers/public/pw_containers/intrusive_list.h b/pw_containers/public/pw_containers/intrusive_list.h
@@ -22,6 +22,7 @@
 #include <type_traits>
 
 #include "pw_containers/config.h"
+#include "pw_containers/internal/intrusive.h"
 #include "pw_containers/internal/intrusive_list.h"
 #include "pw_containers/internal/intrusive_list_item.h"
 #include "pw_containers/internal/intrusive_list_iterator.h"
@@ -112,12 +113,12 @@ class IntrusiveList {
     constexpr explicit Item() = default;
 
    private:
-    // GetListElementTypeFromItem is used to find the element type from an item.
+    // GetElementTypeFromItem is used to find the element type from an item.
     // It is used to ensure list items inherit from the correct Item type.
     template <typename, typename, bool>
-    friend struct ::pw::containers::internal::GetListElementTypeFromItem;
+    friend struct ::pw::containers::internal::GetElementTypeFromItem;
 
-    using PwIntrusiveListElementType = T;
+    using ElementType = T;
   };
 
   using iterator =