pw_allocator: Properly default initialize arrays

This CL default initializes arrays instead of value
initializing arrays in NewArray. This CL also fixes
a small type comparison error in UniquePtr.

Change-Id: Ieceaf222847e980454c7240ee20269d2b6968c62
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/244772
Docs-Not-Needed: Matthew Sedam <[email protected]>
Reviewed-by: Taylor Cramer <[email protected]>
Commit-Queue: Taylor Cramer <[email protected]>
Commit-Queue: Matthew Sedam <[email protected]>
Docs-Not-Needed: Taylor Cramer <[email protected]>
Lint: Lint 🤖 <[email protected]>

pw_allocator/public/pw_allocator/allocator.h

@@ -61,19 +61,11 @@ class Allocator : public Deallocator {
   /// pointer.
   ///
   /// @param[in]  size        The size of the array to allocate.
-  /// @param[in]  args...     Arguments passed to the object constructor.
-  template <typename T, int&... ExplicitGuard, typename... Args>
-  T* NewArray(size_t size, Args&&... args) {
+  template <typename T, int&... ExplicitGuard>
+  T* NewArray(size_t size) {
     Layout layout(sizeof(T) * size, alignof(T));
-    T* ptr = static_cast<T*>(Allocate(layout));
-    if (ptr == nullptr) {
-      return nullptr;
-    }
-
-    for (size_t i = 0; i < size; ++i) {
-      new (ptr + i) T(args...);
-    }
-    return ptr;
+    void* ptr = Allocate(layout);
+    return ptr != nullptr ? new (ptr) T[size] : nullptr;
   }
 
   /// Constructs and object of type `T` from the given `args`, and wraps it in a
@@ -95,11 +87,9 @@ class Allocator : public Deallocator {
   /// fails. Callers must check for null before using the `UniquePtr`.
   ///
   /// @param[in]  size        The size of the array to allocate.
-  /// @param[in]  args...     Arguments passed to the object constructor.
-  template <typename T, int&... ExplicitGuard, typename... Args>
-  [[nodiscard]] UniquePtr<T[]> MakeUniqueArray(size_t size, Args&&... args) {
-    return Deallocator::WrapUniqueArray<T>(
-        NewArray<T>(size, std::forward<Args>(args)...), size);
+  template <typename T, int&... ExplicitGuard>
+  [[nodiscard]] UniquePtr<T[]> MakeUniqueArray(size_t size) {
+    return Deallocator::WrapUniqueArray<T>(NewArray<T>(size), size);
   }
 
   /// Modifies the size of an previously-allocated block of memory without

pw_allocator/public/pw_allocator/unique_ptr.h

@@ -83,7 +83,8 @@ class UniquePtr : public allocator::internal::BaseUniquePtr {
         deallocator_(other.deallocator_),
         size_(other.size_) {
     static_assert(
-        std::is_assignable_v<T*&, U*>,
+        std::is_assignable_v<UnderlyingType*&,
+                             typename UniquePtr<U>::UnderlyingType*>,
         "Attempted to construct a UniquePtr<T> from a UniquePtr<U> where "
         "U* is not assignable to T*.");
     other.Release();
@@ -106,7 +107,8 @@ class UniquePtr : public allocator::internal::BaseUniquePtr {
   /// ``UniquePtr<Base> base = deallocator.MakeUnique<Child>();``.
   template <typename U>
   UniquePtr& operator=(UniquePtr<U>&& other) noexcept {
-    static_assert(std::is_assignable_v<T*&, U*>,
+    static_assert(std::is_assignable_v<UnderlyingType*&,
+                                       typename UniquePtr<U>::UnderlyingType*>,
                   "Attempted to assign a UniquePtr<U> to a UniquePtr<T> where "
                   "U* is not assignable to T*.");
     Reset();

pw_allocator/unique_ptr_test.cc

@@ -152,40 +152,44 @@ TEST_F(UniquePtrTest, DestructorDestroysAndFrees) {
   EXPECT_EQ(allocator_.deallocate_size(), sizeof(DestructorCounter));
 }
 
+class ConstructorCounter {
+ public:
+  ConstructorCounter() { ++count_; }
+
+  size_t getCount() { return count_; }
+
+ private:
+  static size_t count_;
+};
+size_t ConstructorCounter::count_ = 0;
+
 TEST_F(UniquePtrTest, ArrayElementsAreConstructed) {
   constexpr static size_t kArraySize = 5;
-  size_t count = 0;
-  class ConstructorCounter {
-   public:
-    ConstructorCounter(size_t& count) { ++count; }
-  };
 
-  EXPECT_EQ(count, 0ul);
   pw::UniquePtr<ConstructorCounter[]> ptr =
-      allocator_.MakeUniqueArray<ConstructorCounter>(kArraySize, count);
+      allocator_.MakeUniqueArray<ConstructorCounter>(kArraySize);
   ASSERT_NE(ptr, nullptr);
-  EXPECT_EQ(count, kArraySize);
+  EXPECT_EQ(ptr[0].getCount(), kArraySize);
 }
 
+class DestructorCounter {
+ public:
+  ~DestructorCounter() { ++count_; }
+
+  static size_t count_;
+};
+size_t DestructorCounter::count_ = 0;
+
 TEST_F(UniquePtrTest, DestructorDestroysAndFreesArray) {
   constexpr static size_t kArraySize = 5;
-  size_t count = 0;
-  class DestructorCounter {
-   public:
-    DestructorCounter(size_t& count) : count_(&count) {}
-    ~DestructorCounter() { (*count_)++; }
-
-   private:
-    size_t* count_;
-  };
   pw::UniquePtr<DestructorCounter[]> ptr =
-      allocator_.MakeUniqueArray<DestructorCounter>(kArraySize, count);
+      allocator_.MakeUniqueArray<DestructorCounter>(kArraySize);
   ASSERT_NE(ptr, nullptr);
 
-  EXPECT_EQ(count, 0ul);
+  EXPECT_EQ(DestructorCounter::count_, 0ul);
   EXPECT_EQ(allocator_.deallocate_size(), 0ul);
   ptr.Reset();  // Reset the UniquePtr, destroying its contents.
-  EXPECT_EQ(count, kArraySize);
+  EXPECT_EQ(DestructorCounter::count_, kArraySize);
   EXPECT_EQ(allocator_.deallocate_size(),
             sizeof(DestructorCounter) * kArraySize);
 }