Merge pull request #55 from bemanproject/spec-test

Ensure implementation is up to spec

Author: wusatosi

.github/workflows/ci_tests.yml

@@ -37,7 +37,7 @@ jobs:
             c: gcc
           - cpp: clang++
             c: clang
-        cpp_version: [23]
+        cpp_version: [20, 23]
         cmake_args:
           - description: "Default"
             args: ""
@@ -50,15 +50,7 @@ jobs:
             compiler:
               cpp: g++
               c: gcc
-            cpp_version: 23
-            cmake_args:
-              description: "Werror"
-              args: "-DCMAKE_CXX_FLAGS='-Wall -Wextra -Wpedantic -Werror'"
-          - platform: ubuntu-24.04
-            compiler:
-              cpp: g++
-              c: gcc
-            cpp_version: 23
+            cpp_version: 20
             cmake_args:
               description: "Dynamic"
               cmake_args: "-DBUILD_SHARED_LIBS=on"

CMakeLists.txt

@@ -27,6 +27,7 @@ option(
     ${PROJECT_IS_TOP_LEVEL}
 )
 
+include(FetchContent)
 include(GNUInstallDirs)
 
 add_library(beman.inplace_vector INTERFACE)
@@ -59,6 +60,22 @@ install(
 
 if(BEMAN_INPLACE_VECTOR_BUILD_TESTS)
     include(CTest)
+    enable_testing()
+
+    # Fetch GoogleTest
+    FetchContent_Declare(
+        googletest
+        GIT_REPOSITORY https://github.com/google/googletest.git
+        GIT_TAG
+            f8d7d77c06936315286eb55f8de22cd23c188571 # release-1.14.0
+        EXCLUDE_FROM_ALL
+    )
+
+    block()
+        set(INSTALL_GTEST OFF) # Disable GoogleTest installation
+        FetchContent_MakeAvailable(googletest)
+    endblock()
+
     add_subdirectory(tests/beman/inplace_vector)
 endif()
 

include/beman/inplace_vector/inplace_vector.hpp

@@ -313,7 +313,7 @@ template <class = void>
 [[noreturn]]
 static constexpr void __assert_failure(char const *__file, int __line,
                                        char const *__msg) {
-  if consteval {
+  if (std::is_constant_evaluated()) {
     throw __msg; // TODO: std lib implementer, do better here
   } else {
     std::fprintf(stderr, "%s(%d): %s\n", __file, __line, __msg);
@@ -460,7 +460,11 @@ template <class __T, size_t __N> struct __non_trivial {
   constexpr __non_trivial &operator=(__non_trivial const &) noexcept = default;
   constexpr __non_trivial(__non_trivial &&) noexcept = default;
   constexpr __non_trivial &operator=(__non_trivial &&) noexcept = default;
-  constexpr ~__non_trivial() = default;
+
+  constexpr ~__non_trivial()
+    requires(is_trivially_destructible_v<__T>)
+  = default;
+  constexpr ~__non_trivial() { destroy(__data(), __data() + __size()); }
 };
 
 // Selects the vector storage.
@@ -501,7 +505,7 @@ struct inplace_vector : private __iv_detail::__storage::_t<__T, __N> {
   using const_reverse_iterator = ::std::reverse_iterator<const_iterator>;
 
   // [containers.sequences.inplace_vector.cons], construct/copy/destroy
-  constexpr inplace_vector() noexcept { __unsafe_set_size(0); }
+  constexpr inplace_vector() noexcept = default;
   // constexpr explicit inplace_vector(size_type __n);
   // constexpr inplace_vector(size_type __n, const __T& __value);
   // template <class __InputIterator>  // BUGBUG: why not model input_iterator?
@@ -695,11 +699,12 @@ struct inplace_vector : private __iv_detail::__storage::_t<__T, __N> {
   }
 
   template <class... __Args>
-  constexpr void emplace_back(__Args &&...__args)
+  constexpr __T &emplace_back(__Args &&...__args)
     requires(constructible_from<__T, __Args...>)
   {
     if (!try_emplace_back(::std::forward<__Args>(__args)...)) [[unlikely]]
       throw bad_alloc();
+    return back();
   }
   constexpr __T &push_back(const __T &__x)
     requires(constructible_from<__T, const __T &>)
@@ -770,7 +775,8 @@ struct inplace_vector : private __iv_detail::__storage::_t<__T, __N> {
              movable<__T>)
   {
     __assert_iterator_in_range(__position);
-    __assert_valid_iterator_pair(__first, __last);
+    // __assert_valid_iterator_pair(__first, __last); // does not work with
+    // arbitrary InputIterators
     if constexpr (random_access_iterator<__InputIterator>) {
       if (size() + static_cast<size_type>(distance(__first, __last)) >
           capacity()) [[unlikely]]
@@ -935,27 +941,60 @@ struct inplace_vector : private __iv_detail::__storage::_t<__T, __N> {
   }
 
   constexpr inplace_vector(const inplace_vector &__x)
-    requires(copyable<__T>)
+    requires(__N == 0 || is_trivially_copy_constructible_v<__T>)
+  = default;
+
+  constexpr inplace_vector(const inplace_vector &__x)
+    requires(__N != 0 && !is_trivially_copy_constructible_v<__T> &&
+             copyable<__T>)
   {
     for (auto &&__e : __x)
       emplace_back(__e);
   }
+
   constexpr inplace_vector(inplace_vector &&__x)
-    requires(movable<__T>)
+    requires(__N == 0 || is_trivially_move_constructible_v<__T>)
+  = default;
+
+  constexpr inplace_vector(inplace_vector &&__x)
+    requires(__N != 0 && !is_trivially_move_constructible_v<__T> &&
+             movable<__T>)
   {
     for (auto &&__e : __x)
       emplace_back(::std::move(__e));
   }
+
   constexpr inplace_vector &operator=(const inplace_vector &__x)
-    requires(copyable<__T>)
+    requires(__N == 0 || (std::is_trivially_destructible_v<__T> &&
+                          std::is_trivially_copy_constructible_v<__T> &&
+                          std::is_trivially_copy_assignable_v<__T>))
+  = default;
+
+  constexpr inplace_vector &operator=(const inplace_vector &__x)
+    requires(__N != 0 &&
+             !(std::is_trivially_destructible_v<__T> &&
+               std::is_trivially_copy_constructible_v<__T> &&
+               std::is_trivially_copy_assignable_v<__T>) &&
+             copyable<__T>)
   {
     clear();
     for (auto &&__e : __x)
       emplace_back(__e);
     return *this;
   }
+
   constexpr inplace_vector &operator=(inplace_vector &&__x)
-    requires(movable<__T>)
+    requires(__N == 0 || (std::is_trivially_destructible_v<__T> &&
+                          std::is_trivially_move_constructible_v<__T> &&
+                          std::is_trivially_move_assignable_v<__T>))
+  = default;
+
+  constexpr inplace_vector &operator=(inplace_vector &&__x)
+    requires(__N != 0 &&
+             !(std::is_trivially_destructible_v<__T> &&
+               std::is_trivially_move_constructible_v<__T> &&
+               std::is_trivially_move_assignable_v<__T>) &&
+             movable<__T>)
   {
     clear();
     for (auto &&__e : __x)
@@ -986,7 +1025,7 @@ struct inplace_vector : private __iv_detail::__storage::_t<__T, __N> {
     requires(constructible_from<__T, ranges::range_reference_t<__R>> &&
              movable<__T>)
   {
-    assign(begin(__rg), end(__rg));
+    assign(std::begin(__rg), std::end(__rg));
   }
   constexpr void assign(size_type __n, const __T &__u)
     requires(constructible_from<__T, const __T &> && movable<__T>)

tests/beman/inplace_vector/CMakeLists.txt

@@ -20,3 +20,23 @@ add_test(
     NAME beman.inplace_vector.ref-test
     COMMAND beman.inplace_vector.ref-test
 )
+
+# GoogleTest based tests
+include(GoogleTest)
+
+function(add_gtest NAME)
+    add_executable(beman.inplace_vector.tests.${NAME})
+    target_sources(beman.inplace_vector.tests.${NAME} PRIVATE ${NAME}.test.cpp)
+    target_link_libraries(
+        beman.inplace_vector.tests.${NAME}
+        PRIVATE beman.inplace_vector GTest::gtest GTest::gtest_main
+    )
+    gtest_add_tests(beman.inplace_vector.tests.${NAME} "" AUTO)
+endfunction()
+
+# Tests for official specs
+add_gtest(container_requirements)
+add_gtest(triviality)
+add_gtest(constructors)
+add_gtest(size_n_data)
+add_gtest(erasure)

tests/beman/inplace_vector/README.md

@@ -0,0 +1,109 @@
+# Inplace Vector Testing Suite
+
+This folder contains tests for `inplace_vector` implementation.
+
+The aim for the test cases are to keep the implementation in-check with its standing in the latest C++ draft.
+
+You can checkout `inplace_vector`'s current state in C++ draft [here](https://eel.is/c++draft/inplace.vector).
+
+## C++ Draft paragraph to test file
+
+`inplace_vector`'s chapter number in the latest C++ draft is **23.3.14**.
+
+### Overview (23.3.14.1)
+
+[Overview](https://eel.is/c++draft/inplace.vector#overview) has 6 clauses.
+
+#### 6.1 Overview
+
+> An inplace_vector is a contiguous container. Its capacity is fixed and its
+elements are stored within the inplace_vector object itself.
+
+This is not testable.
+
+#### 6.2 Container Requirements
+
+> An inplace_vector meets all of the requirements of a container ([container.reqmts]),
+> of a reversible container ([container.rev.reqmts]), of a contiguous container,
+> and of a sequence container, including most of the optional sequence
+container requirements ([sequence.reqmts]). The exceptions are the push_front,
+> prepend_range, pop_front, and emplace_front member functions,
+> which are not provided.
+> Descriptions are provided here only for operations on inplace_vector that
+> are not described in one of these tables or for operations where there is
+> additional semantic information.
+
+See [container_requirements.test.cpp](container_requirements.test.cpp).
+
+#### 6.3 Constexpr Iterator
+
+> For any N, `inplace_vector<T, N>​::​iterator` and
+> `inplace_vector<T, N>​::​const_iterator` meet the constexpr iterator
+> requirements.
+
+Not tested for now.
+
+#### 6.4 Constexpr member functions
+
+> For any $N>0$, if T is not trivially copyable or
+> `is_trivially_default_constructible_v<T>` is false,
+> then no `inplace_vector<T, N>` member functions are usable in
+> constant expressions.
+
+Not tested for now.
+
+#### 6.5 Bad alloc requirement
+
+> Any member function of `inplace_vector<T, N>` that would cause the size to
+> exceed N throws an exception of type bad_alloc.
+
+These are tested with individual functions.
+
+#### 6.6 Triviality
+
+Let IV denote a specialization of `inplace_vector<T, N>`.
+> If N is zero, then IV is trivially copyable and empty,
+> and std​::​is_trivially_default_constructible_v<IV> is true.
+> (Sub-clauses omitted)
+
+See [triviality.test.cpp](triviality.test.cpp)
+
+### Constructors (23.3.14.2)
+
+See [constructors.test.cpp](constructors.test.cpp)
+
+### Size and capacity (23.3.14.3)
+
+See [size_n_data.test.cpp](size_n_data.test.cpp)
+
+### Data (23.3.14.4)
+
+See [size_n_data.test.cpp](size_n_data.test.cpp)
+
+### Modifiers (23.3.14.5)
+
+See [modifiers.test.cpp](modifiers.test.cpp)
+
+### Erasure (23.3.14.6)
+
+See [erasure.test.cpp](erasure.test.cpp)
+
+## Other tests
+
+- [ref_impl.test.cpp](ref_impl.test.cpp):
+  Is the test suite imported from reference implementation in
+  [P0843R14](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2024/p0843r14.html).
+  Originally included [here on godbolt](https://godbolt.org/z/5P78aG5xE).
+- [inplace_vector.test.cpp](inplace_vector.test.cpp):
+  A minimal test suite by @Hels15 for their implementation.
+
+## Known Issues/ Missed Tests
+
+- Constexpr related functionalities.
+- Emplacement minimal copy/ construction.
+- Exception safety on mutation.
+
+## Special Thanks
+
+Special thanks to Jan Babst (@jbab) for his contribution at setting up the
+Google Test infrastructure.