pw_containers: Fix InlineQueue and InlineDeque copy methods
This CL adds tests for the copy constructors and copy assignment
operators of InlineDeque and InlineQueue. It fixes typos and bugs
revealed by the added tests.
It also adds them as deps of `//pw_containers::pw_containers` to allow
consumers of //pw_containers to access them more easily.
Change-Id: I15c7ece01293a95af9bb0b3a13ba3da541db16a2
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/148335
Reviewed-by: Taylor Cramer <cramertj@google.com>
Commit-Queue: Aaron Green <aarongreen@google.com>
diff --git a/pw_containers/BUILD.bazel b/pw_containers/BUILD.bazel
index 5d82f2e..b0799cb 100644
--- a/pw_containers/BUILD.bazel
+++ b/pw_containers/BUILD.bazel
@@ -27,6 +27,8 @@
deps = [
":algorithm",
":flat_map",
+ ":inline_deque",
+ ":inline_queue",
":intrusive_list",
":vector",
],
@@ -246,6 +248,7 @@
],
deps = [
":intrusive_list",
+ ":vector",
"//pw_unit_test",
],
)
diff --git a/pw_containers/BUILD.gn b/pw_containers/BUILD.gn
index e144eb6..a6bbe08 100644
--- a/pw_containers/BUILD.gn
+++ b/pw_containers/BUILD.gn
@@ -28,6 +28,8 @@
public_deps = [
":algorithm",
":flat_map",
+ ":inline_deque",
+ ":inline_queue",
":intrusive_list",
":vector",
]
diff --git a/pw_containers/CMakeLists.txt b/pw_containers/CMakeLists.txt
index 6646979..ef8a09f 100644
--- a/pw_containers/CMakeLists.txt
+++ b/pw_containers/CMakeLists.txt
@@ -18,6 +18,8 @@
PUBLIC_DEPS
pw_containers.algorithm
pw_containers.flat_map
+ pw_containers.inline_deque
+ pw_containers.inline_queue
pw_containers.intrusive_list
pw_containers.vector
)
diff --git a/pw_containers/inline_deque_test.cc b/pw_containers/inline_deque_test.cc
index 0e0cc88..b78ece5 100644
--- a/pw_containers/inline_deque_test.cc
+++ b/pw_containers/inline_deque_test.cc
@@ -46,6 +46,39 @@
EXPECT_EQ(deque.max_size(), 3u);
}
+TEST(InlineDeque, Construct_CopySameCapacity) {
+ InlineDeque<CopyOnly, 4> deque(4, CopyOnly(123));
+ InlineDeque<CopyOnly, 4> copied(deque);
+
+ EXPECT_EQ(4u, deque.size());
+ EXPECT_EQ(123, deque[3].value);
+
+ EXPECT_EQ(4u, copied.size());
+ EXPECT_EQ(123, copied[3].value);
+}
+
+TEST(InlineDeque, Construct_CopyLargerCapacity) {
+ InlineDeque<CopyOnly, 4> deque(4, CopyOnly(123));
+ InlineDeque<CopyOnly, 5> copied(deque);
+
+ EXPECT_EQ(4u, deque.size());
+ EXPECT_EQ(123, deque[3].value);
+
+ EXPECT_EQ(4u, copied.size());
+ EXPECT_EQ(123, copied[3].value);
+}
+
+TEST(InlineDeque, Construct_CopySmallerCapacity) {
+ InlineDeque<CopyOnly, 4> deque(3, CopyOnly(123));
+ InlineDeque<CopyOnly, 3> copied(deque);
+
+ EXPECT_EQ(3u, deque.size());
+ EXPECT_EQ(123, deque[2].value);
+
+ EXPECT_EQ(3u, copied.size());
+ EXPECT_EQ(123, copied[2].value);
+}
+
TEST(InlineDeque, Destruct_ZeroLength) {
Counter::Reset();
{
@@ -67,7 +100,7 @@
EXPECT_EQ(Counter::destroyed, 0);
}
-TEST(InlineDeque, Destruct_MultpileEntries) {
+TEST(InlineDeque, Destruct_MultipleEntries) {
Counter value;
Counter::Reset();
@@ -88,6 +121,39 @@
EXPECT_EQ(7, deque[3]);
}
+TEST(InlineDeque, Assign_CopySameCapacity) {
+ InlineDeque<CopyOnly, 4> deque(4, CopyOnly(123));
+ InlineDeque<CopyOnly, 4> copied = deque;
+
+ EXPECT_EQ(4u, deque.size());
+ EXPECT_EQ(123, deque[3].value);
+
+ EXPECT_EQ(4u, copied.size());
+ EXPECT_EQ(123, copied[3].value);
+}
+
+TEST(InlineDeque, Assign_CopyLargerCapacity) {
+ InlineDeque<CopyOnly, 4> deque(4, CopyOnly(123));
+ InlineDeque<CopyOnly, 5> copied = deque;
+
+ EXPECT_EQ(4u, deque.size());
+ EXPECT_EQ(123, deque[3].value);
+
+ EXPECT_EQ(4u, copied.size());
+ EXPECT_EQ(123, copied[3].value);
+}
+
+TEST(InlineDeque, Assign_CopySmallerCapacity) {
+ InlineDeque<CopyOnly, 4> deque(3, CopyOnly(123));
+ InlineDeque<CopyOnly, 3> copied = deque;
+
+ EXPECT_EQ(3u, deque.size());
+ EXPECT_EQ(123, deque[2].value);
+
+ EXPECT_EQ(3u, copied.size());
+ EXPECT_EQ(123, copied[2].value);
+}
+
TEST(InlineDeque, Access_Iterator) {
InlineDeque<Counter, 2> deque(2);
for (Counter& item : deque) {
diff --git a/pw_containers/inline_queue_test.cc b/pw_containers/inline_queue_test.cc
index 857f1d1..5c4392c 100644
--- a/pw_containers/inline_queue_test.cc
+++ b/pw_containers/inline_queue_test.cc
@@ -46,6 +46,39 @@
EXPECT_EQ(queue.max_size(), 3u);
}
+TEST(InlineQueue, Construct_CopySameCapacity) {
+ InlineQueue<CopyOnly, 4> queue(4, CopyOnly(123));
+ InlineQueue<CopyOnly, 4> copied(queue);
+
+ EXPECT_EQ(4u, queue.size());
+ EXPECT_EQ(123, queue[3].value);
+
+ EXPECT_EQ(4u, copied.size());
+ EXPECT_EQ(123, copied[3].value);
+}
+
+TEST(InlineQueue, Construct_CopyLargerCapacity) {
+ InlineQueue<CopyOnly, 4> queue(4, CopyOnly(123));
+ InlineQueue<CopyOnly, 5> copied(queue);
+
+ EXPECT_EQ(4u, queue.size());
+ EXPECT_EQ(123, queue[3].value);
+
+ EXPECT_EQ(4u, copied.size());
+ EXPECT_EQ(123, copied[3].value);
+}
+
+TEST(InlineQueue, Construct_CopySmallerCapacity) {
+ InlineQueue<CopyOnly, 4> queue(3, CopyOnly(123));
+ InlineQueue<CopyOnly, 3> copied(queue);
+
+ EXPECT_EQ(3u, queue.size());
+ EXPECT_EQ(123, queue[2].value);
+
+ EXPECT_EQ(3u, copied.size());
+ EXPECT_EQ(123, copied[2].value);
+}
+
TEST(InlineQueue, Destruct_ZeroLength) {
Counter::Reset();
{
@@ -67,7 +100,7 @@
EXPECT_EQ(Counter::destroyed, 0);
}
-TEST(InlineQueue, Destruct_MultpileEntries) {
+TEST(InlineQueue, Destruct_MultipleEntries) {
Counter value;
Counter::Reset();
@@ -78,7 +111,7 @@
}
TEST(InlineQueue, Assign_InitializerList) {
- InlineQueue<int, 4> queue({1, 3, 5, 7});
+ InlineQueue<int, 4> queue = {1, 3, 5, 7};
EXPECT_EQ(4u, queue.size());
@@ -88,6 +121,39 @@
EXPECT_EQ(7, queue[3]);
}
+TEST(InlineQueue, Assign_CopySameCapacity) {
+ InlineQueue<CopyOnly, 4> queue(4, CopyOnly(123));
+ InlineQueue<CopyOnly, 4> copied = queue;
+
+ EXPECT_EQ(4u, queue.size());
+ EXPECT_EQ(123, queue[3].value);
+
+ EXPECT_EQ(4u, copied.size());
+ EXPECT_EQ(123, copied[3].value);
+}
+
+TEST(InlineQueue, Assign_CopyLargerCapacity) {
+ InlineQueue<CopyOnly, 4> queue(4, CopyOnly(123));
+ InlineQueue<CopyOnly, 5> copied = queue;
+
+ EXPECT_EQ(4u, queue.size());
+ EXPECT_EQ(123, queue[3].value);
+
+ EXPECT_EQ(4u, copied.size());
+ EXPECT_EQ(123, copied[3].value);
+}
+
+TEST(InlineQueue, Assign_CopySmallerCapacity) {
+ InlineQueue<CopyOnly, 4> queue(3, CopyOnly(123));
+ InlineQueue<CopyOnly, 3> copied = queue;
+
+ EXPECT_EQ(3u, queue.size());
+ EXPECT_EQ(123, queue[2].value);
+
+ EXPECT_EQ(3u, copied.size());
+ EXPECT_EQ(123, copied[2].value);
+}
+
TEST(InlineQueue, Access_Iterator) {
InlineQueue<Counter, 2> queue(2);
for (Counter& item : queue) {
diff --git a/pw_containers/public/pw_containers/inline_deque.h b/pw_containers/public/pw_containers/inline_deque.h
index 5dfef69..dfd3227 100644
--- a/pw_containers/public/pw_containers/inline_deque.h
+++ b/pw_containers/public/pw_containers/inline_deque.h
@@ -96,30 +96,37 @@
Base::assign(start, finish);
}
- template <size_type kOtherCapacity>
- BasicInlineDeque(
- const BasicInlineDeque<value_type, size_type, kOtherCapacity>& other)
- : BasicInlineDeque(other.start(), other.end()) {
- static_assert(kCapacity >= kOtherCapacity);
+ BasicInlineDeque(std::initializer_list<value_type> list)
+ : BasicInlineDeque() {
+ *this = list;
}
- BasicInlineDeque(std::initializer_list<value_type> list)
- : BasicInlineDeque(list.begin(), list.end()) {}
+ BasicInlineDeque(const BasicInlineDeque& other) : BasicInlineDeque() {
+ *this = other;
+ }
+
+ template <typename T, typename = containers::internal::EnableIfIterable<T>>
+ BasicInlineDeque(const T& other) : BasicInlineDeque() {
+ *this = other;
+ }
// Assignment
+ // Use the operators from the base class, but return the correct type of
+ // reference.
- BasicInlineDeque& operator=(const BasicInlineDeque& other) = default;
-
- // Checks capacity rather than current size.
- template <size_type kOtherCapacity>
- BasicInlineDeque& operator=(
- const BasicInlineDeque<value_type, size_type, kOtherCapacity>& other) {
- Base::template assign<kOtherCapacity>(other);
+ BasicInlineDeque& operator=(std::initializer_list<value_type> list) {
+ Base::operator=(list);
return *this;
}
- BasicInlineDeque& operator=(std::initializer_list<value_type> list) {
- Base::assign(list);
+ BasicInlineDeque& operator=(const BasicInlineDeque& other) {
+ Base::operator=(other);
+ return *this;
+ }
+
+ template <typename T, typename = containers::internal::EnableIfIterable<T>>
+ BasicInlineDeque& operator=(const T& other) {
+ Base::operator=(other);
return *this;
}
@@ -185,6 +192,17 @@
return *this;
}
+ BasicInlineDeque& operator=(const BasicInlineDeque& other) {
+ assign(other.begin(), other.end());
+ return *this;
+ }
+
+ template <typename T, typename = containers::internal::EnableIfIterable<T>>
+ BasicInlineDeque& operator=(const T& other) {
+ assign(other.begin(), other.end());
+ return *this;
+ }
+
void assign(size_type count, const value_type& value) {
clear();
Append(count, value);
diff --git a/pw_containers/public/pw_containers/inline_queue.h b/pw_containers/public/pw_containers/inline_queue.h
index da8157d..8c43a78 100644
--- a/pw_containers/public/pw_containers/inline_queue.h
+++ b/pw_containers/public/pw_containers/inline_queue.h
@@ -72,25 +72,31 @@
BasicInlineQueue(InputIterator start, InputIterator finish)
: deque_(start, finish) {}
- template <size_type kOtherCapacity>
- BasicInlineQueue(
- const BasicInlineQueue<value_type, size_type, kOtherCapacity>& other)
- : deque_(other) {}
+ BasicInlineQueue(std::initializer_list<value_type> list) { *this = list; }
- BasicInlineQueue(std::initializer_list<value_type> list)
- : BasicInlineQueue(list.begin(), list.end()) {}
+ BasicInlineQueue(const BasicInlineQueue& other) { *this = other; }
+
+ template <typename T, typename = containers::internal::EnableIfIterable<T>>
+ BasicInlineQueue(const T& other) {
+ *this = other;
+ }
// Assignment
- BasicInlineQueue& operator=(const BasicInlineQueue& other) = default;
+ BasicInlineQueue& operator=(std::initializer_list<value_type> list) {
+ deque_ = std::move(list);
+ return *this;
+ }
- // Checks capacity rather than current size.
- template <size_type kOtherCapacity>
- BasicInlineQueue& operator=(
- const BasicInlineQueue<value_type, size_type, kOtherCapacity>& other) {
- deque_ =
- static_cast<BasicInlineDeque<value_type, size_type, kOtherCapacity>>(
- other);
+ BasicInlineQueue& operator=(const BasicInlineQueue& other) {
+ deque_ = other.deque_;
+ return *this;
+ }
+
+ template <typename T, typename = containers::internal::EnableIfIterable<T>>
+ BasicInlineQueue& operator=(const T& other) {
+ deque_ = BasicInlineDeque<ValueType, SizeType, kCapacity>(other.begin(),
+ other.end());
return *this;
}
diff --git a/pw_containers/public/pw_containers/internal/raw_storage.h b/pw_containers/public/pw_containers/internal/raw_storage.h
index 964eeb6..8ec9807 100644
--- a/pw_containers/public/pw_containers/internal/raw_storage.h
+++ b/pw_containers/public/pw_containers/internal/raw_storage.h
@@ -28,6 +28,10 @@
typename std::iterator_traits<InputIterator>::iterator_category,
std::input_iterator_tag>::value>;
+template <typename T>
+using EnableIfIterable =
+ std::enable_if_t<true, decltype(T().begin(), T().end())>;
+
// The DestructorHelper is used to make a Container<T> trivially destructible if
// T is. This could be replaced with a C++20 constraint.
template <typename Container, bool kIsTriviallyDestructible>
