Fix static vector bug (#144)

* pop_back, clear and destructor now all call destructors of elements

* Updated CI script for MacOS

* Added unit tests for static_vector

* Replaced unique_ptr in test with a shared_ptr

Author: glynos

.github/workflows/skyr-url-ci.yml

@@ -270,9 +270,9 @@ jobs:
           chmod +x llvm.sh
           sudo ./llvm.sh 10
 
-      - name: Install vcpkg (Linux/MacOS)
-        id: vcpkg_linux_macos
-        if: startsWith(matrix.config.os, 'ubuntu') || startsWith(matrix.config.os, 'macos')
+      - name: Install vcpkg (Linux)
+        id: vcpkg_linux
+        if: startsWith(matrix.config.os, 'ubuntu')
         shell: bash
         run: |
           mkdir -p ${GITHUB_WORKSPACE}/vcpkg
@@ -285,6 +285,22 @@ jobs:
           ./bootstrap-vcpkg.sh
           ./vcpkg install tl-expected range-v3 catch2 nlohmann-json fmt
 
+      - name: Install vcpkg (MacOS)
+        id: vcpkg_macos
+        if: startsWith(matrix.config.os, 'macos')
+        shell: bash
+        run: |
+          brew install pkg-config
+          mkdir -p ${GITHUB_WORKSPACE}/vcpkg
+          cd ${GITHUB_WORKSPACE}/vcpkg
+          git init
+          git remote add origin https://github.com/microsoft/vcpkg.git
+          git fetch origin master
+          git checkout -b master origin/master
+          export
+          ./bootstrap-vcpkg.sh
+          ./vcpkg install tl-expected range-v3 catch2 nlohmann-json fmt
+
       - name: Install vcpkg (Windows)
         id: vcpkg_windows
         if: startsWith(matrix.config.os, 'windows')

include/skyr/v1/containers/static_vector.hpp

@@ -52,6 +52,10 @@ class static_vector {
   /// Constructor
   constexpr static_vector() = default;
 
+  ~static_vector() {
+    clear();
+  }
+
   /// Gets the first const element in the vector
   /// \return a const T &
   /// \pre `size() > 0`
@@ -83,7 +87,7 @@ class static_vector {
   ///
   /// \param value
   /// \return
-  /// \pre `size() < `capacity()`
+  /// \pre `size() < capacity()`
   /// \post `size() > 0 && size() <= capacity()`
   constexpr auto push_back(const_reference value) noexcept -> reference {
     impl_[size_++] = value;
@@ -94,19 +98,20 @@ class static_vector {
   /// \tparam Args
   /// \param args
   /// \return
-  /// \pre `size() < `capacity()`
+  /// \pre `size() < capacity()`
   /// \post `size() > 0 && size() <= capacity()`
   template <class... Args>
   constexpr auto emplace_back(Args &&... args)
     noexcept(std::is_trivially_move_assignable_v<T>) -> reference {
-    impl_[size_++] = value_type{args...};
+    impl_[size_++] = value_type{std::forward<Args>(args)...};
     return impl_[size_ - 1];
   }
 
   ///
   /// \pre `size() > 0`
   constexpr void pop_back() noexcept {
     --size_;
+    impl_[size_].~T();
   }
 
   ///
@@ -140,8 +145,11 @@ class static_vector {
   }
 
   ///
+  /// \post size() == 0
   constexpr void clear() noexcept {
-    size_ = 0;
+    while (size_) {
+      pop_back();
+    }
   }
 
   ///

tests/CMakeLists.txt

@@ -33,6 +33,7 @@ function(skyr_create_test file_name output_dir test_name)
     set(test_name ${test} PARENT_SCOPE)
 endfunction()
 
+add_subdirectory(containers)
 add_subdirectory(unicode)
 add_subdirectory(domain)
 add_subdirectory(percent_encoding)

tests/containers/CMakeLists.txt

@@ -0,0 +1,10 @@
+# Copyright (c) Glyn Matthews 2020.
+# Distributed under the Boost Software License, Version 1.0.
+# (See accompanying file LICENSE_1_0.txt or copy at
+# http://www.boost.org/LICENSE_1_0.txt)
+
+foreach (
+        file_name
+        static_vector_tests.cpp)
+    skyr_create_test(${file_name} ${PROJECT_BINARY_DIR}/tests/container test_name)
+endforeach ()

tests/containers/static_vector_tests.cpp

@@ -0,0 +1,48 @@
+// Copyright 2020 Glyn Matthews.
+// Distributed under the Boost Software License, Version 1.0.
+// (See accompanying file LICENSE_1_0.txt or copy at
+// http://www.boost.org/LICENSE_1_0.txt)
+
+#include <memory>
+#define CATCH_CONFIG_MAIN
+#include <catch2/catch.hpp>
+#include <skyr/v1/containers/static_vector.hpp>
+
+
+struct test_destructor_call {
+  bool *destructed = nullptr;
+
+  explicit test_destructor_call(bool *destructed)
+  : destructed(destructed) {
+    *destructed = false;
+  }
+
+  test_destructor_call(const test_destructor_call &) = delete;
+  test_destructor_call &operator=(const test_destructor_call &) = delete;
+  test_destructor_call(test_destructor_call &&) = delete;
+  test_destructor_call &operator=(test_destructor_call &&) = delete;
+
+  ~test_destructor_call() {
+    *destructed = true;
+  }
+};
+
+TEST_CASE("pop back calls destructor", "[containers]") {
+  auto destructed = false;
+
+  auto vector = skyr::static_vector<std::shared_ptr<test_destructor_call>, 8>{};
+  vector.emplace_back(std::make_shared<test_destructor_call>(&destructed));
+  CHECK_FALSE(destructed);
+  vector.pop_back();
+  CHECK(destructed);
+}
+
+TEST_CASE("clear calls destructor", "[containers]") {
+  auto destructed = false;
+
+  auto vector = skyr::static_vector<std::shared_ptr<test_destructor_call>, 8>{};
+  vector.emplace_back(std::make_shared<test_destructor_call>(&destructed));
+  CHECK_FALSE(destructed);
+  vector.clear();
+  CHECK(destructed);
+}