c++: remove assertion std::is_base_of_v<> (#6)

Because Derived is not a complete type at the time of the check, the behavior of std::is_base_of<> is undefined; see https://en.cppreference.com/w/cpp/types/is_base_of.

Add missing includes to the test application and bump the CI deps.

.github/workflows/main.yml

@@ -10,16 +10,16 @@ jobs:
         toolchain: ['clang', 'gcc']
         include:
           - toolchain: gcc
-            c-compiler: gcc
-            cxx-compiler: g++
+            c-compiler: gcc-12
+            cxx-compiler: g++-12
           - toolchain: clang
             c-compiler: clang
             cxx-compiler: clang++
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
 
       - name: Install Dependencies
-        run: sudo apt install gcc-multilib g++-multilib clang-tidy-12
+        run: sudo apt install gcc-multilib g++-multilib clang-tidy-14
 
       - name: Configure CMake
         run: >
@@ -36,7 +36,7 @@ jobs:
 
       - name: Test
         working-directory: ${{github.workspace}}/build
-        run: make test
+        run: make test ARGS="--verbose"
 
   optimizations:
     runs-on: ubuntu-latest
@@ -46,13 +46,13 @@ jobs:
         build_type: [Release, MinSizeRel]
         include:
           - toolchain: gcc
-            c-compiler: gcc
-            cxx-compiler: g++
+            c-compiler: gcc-12
+            cxx-compiler: g++-12
           - toolchain: clang
             c-compiler: clang
             cxx-compiler: clang++
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
 
       - name: Install Dependencies
         run: sudo apt install gcc-multilib g++-multilib
@@ -73,15 +73,15 @@ jobs:
 
       - name: Test
         working-directory: ${{github.workspace}}/build
-        run: make test
+        run: make test ARGS="--verbose"
 
   style_check:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
-      - uses: DoozyX/clang-format-lint-action@v0.12
+      - uses: actions/checkout@v3
+      - uses: DoozyX/clang-format-lint-action@v0.14
         with:
           source: '.'
           exclude: './unity'
           extensions: 'c,h,cpp,hpp'
-          clangFormatVersion: 12
+          clangFormatVersion: 14

CMakeLists.txt

@@ -10,19 +10,23 @@ set(CTEST_OUTPUT_ON_FAILURE ON)
 
 # Use -DNO_STATIC_ANALYSIS=1 to suppress static analysis. If not suppressed, the tools used here shall be available.
 if (NOT NO_STATIC_ANALYSIS)
-    find_program(clang_tidy NAMES clang-tidy-12 clang-tidy)
+    find_program(clang_tidy NAMES clang-tidy-14 clang-tidy)
     if (NOT clang_tidy)
         message(FATAL_ERROR "Could not locate clang-tidy")
     endif ()
     message(STATUS "Using clang-tidy: ${clang_tidy}")
     set(CMAKE_CXX_CLANG_TIDY ${clang_tidy})
 endif ()
 
-find_program(clang_format NAMES clang-format-12 clang-format)
+find_program(clang_format NAMES clang-format-14 clang-format)
 if (NOT clang_format)
     message(STATUS "Could not locate clang-format")
 else ()
-    file(GLOB format_files ${CMAKE_CURRENT_SOURCE_DIR}/*.[ch] ${CMAKE_CURRENT_SOURCE_DIR}/*.[ch]pp)
+    file(GLOB format_files
+         ${CMAKE_CURRENT_SOURCE_DIR}/c/*.[ch]pp
+         ${CMAKE_CURRENT_SOURCE_DIR}/c/*.[ch]
+         ${CMAKE_CURRENT_SOURCE_DIR}/c++/*.[ch]pp
+    )
     message(STATUS "Using clang-format: ${clang_format}; files: ${format_files}")
     add_custom_target(format COMMAND ${clang_format} -i -fallback-style=none -style=file --verbose ${format_files})
 endif ()
@@ -43,6 +47,13 @@ target_include_directories(unity SYSTEM PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/unity
 target_compile_definitions(unity PUBLIC -DUNITY_SHORTHAND_AS_RAW=1 -DUNITY_OUTPUT_COLOR=1)
 
 add_executable(test_c ${CMAKE_CURRENT_SOURCE_DIR}/c/test.cpp)
+if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
+    # GCC miscompiles the test with optimizations enabled with the strict aliasing enabled
+    # due to the reinterpret_cast<> in a test helper. This should be fixed one day.
+    # This deficiency of the test suite is not expected to affect applications.
+    # This workaround is also not required for clang.
+    target_compile_options(test_c PRIVATE -fno-strict-aliasing)
+endif()
 target_link_libraries(test_c unity)
 add_test("run_test_c" "test_c")
 

c++/.clang-tidy

@@ -20,6 +20,7 @@ Checks: >-
   -readability-magic-numbers,
   -readability-function-size,
   -readability-function-cognitive-complexity,
+  -readability-identifier-length,
   -llvm-header-guard,
   -llvm-include-order,
   -cppcoreguidelines-avoid-magic-numbers,

c++/cavl.hpp

@@ -57,13 +57,13 @@ class Node
     using DerivedType = Derived;
 
     // Tree nodes cannot be copied for obvious reasons.
-    Node(const Node&) = delete;
+    Node(const Node&)                    = delete;
     auto operator=(const Node&) -> Node& = delete;
 
     // They can't be moved either, but the reason is less obvious.
     // While we can trivially update the pointers in the adjacent nodes to keep the tree valid,
     // we can't update external references to the tree. This breaks the tree if one attempted to move its root node.
-    Node(Node&& other) = delete;
+    Node(Node&& other)                    = delete;
     auto operator=(Node&& other) -> Node& = delete;
 
 protected:
@@ -420,7 +420,7 @@ auto Node<Derived>::adjustBalance(const bool increment) noexcept -> Node*
         const bool   r    = new_bf < 0;   // bf<0 if left-heavy --> right rotation is needed.
         const int8_t sign = r ? +1 : -1;  // Positive if we are rotating right.
         Node* const  z    = lr[!r];
-        CAVL_ASSERT(z != nullptr);  // Heavy side cannot be empty.
+        CAVL_ASSERT(z != nullptr);  // Heavy side cannot be empty. NOLINTNEXTLINE(clang-analyzer-core.NullDereference)
         if ((z->bf * sign) <= 0)    // Parent and child are heavy on the same side or the child is balanced.
         {
             out = z;
@@ -508,7 +508,7 @@ class Tree final
     ~Tree() = default;
 
     /// Trees cannot be copied.
-    Tree(const Tree&) = delete;
+    Tree(const Tree&)                    = delete;
     auto operator=(const Tree&) -> Tree& = delete;
 
     /// Trees can be easily moved in constant time. This does not actually affect the tree itself, only this object.
@@ -603,7 +603,6 @@ class Tree final
     auto empty() const noexcept { return root_ == nullptr; }
 
 private:
-    static_assert(std::is_base_of_v<NodeType, Derived>, "Invalid usage: CRTP inheritance required");
     static_assert(!std::is_polymorphic_v<NodeType>);
     static_assert(std::is_same_v<Tree<Derived>, typename NodeType::TreeType>);
 
@@ -617,10 +616,10 @@ class Tree final
         explicit TraversalIndicatorUpdater(const Tree& sup) noexcept : that(sup) { that.traversal_in_progress_ = true; }
         ~TraversalIndicatorUpdater() noexcept { that.traversal_in_progress_ = false; }
 
-        TraversalIndicatorUpdater(const TraversalIndicatorUpdater&) = delete;
-        TraversalIndicatorUpdater(TraversalIndicatorUpdater&&)      = delete;
+        TraversalIndicatorUpdater(const TraversalIndicatorUpdater&)                    = delete;
+        TraversalIndicatorUpdater(TraversalIndicatorUpdater&&)                         = delete;
         auto operator=(const TraversalIndicatorUpdater&) -> TraversalIndicatorUpdater& = delete;
-        auto operator=(TraversalIndicatorUpdater&&) -> TraversalIndicatorUpdater& = delete;
+        auto operator=(TraversalIndicatorUpdater&&) -> TraversalIndicatorUpdater&      = delete;
 
     private:
         const Tree& that;

c++/test.cpp

@@ -12,6 +12,8 @@
 #include <sstream>
 #include <memory>
 #include <numeric>
+#include <vector>
+#include <functional>
 
 void setUp() {}
 
@@ -199,7 +201,7 @@ void testManual(const std::function<N*(std::uint8_t)>& factory)
     TreeType tr;
     TEST_ASSERT(tr.empty());
     const auto insert = [&](const std::uint8_t i) {
-        std::cout << "Inserting " << int(i) << std::endl;
+        std::cout << "Inserting " << static_cast<int>(i) << std::endl;
         const auto pred = [&](const N& v) { return t.at(i)->getValue() - v.getValue(); };
         TEST_ASSERT_NULL(tr.search(pred));
         TEST_ASSERT_NULL(static_cast<const TreeType&>(tr).search(pred));
@@ -789,12 +791,12 @@ class V : public cavl::Node<V>
     using Self::min;
     using Self::max;
 
-    V()          = default;
-    virtual ~V() = default;
-    V(const V&)  = delete;
-    V(V&&)       = delete;
+    V()                    = default;
+    virtual ~V()           = default;
+    V(const V&)            = delete;
+    V(V&&)                 = delete;
     V& operator=(const V&) = delete;
-    V& operator=(V&&) = delete;
+    V& operator=(V&&)      = delete;
 
     [[nodiscard]] virtual auto getValue() const -> std::uint16_t = 0;
 

c/.clang-tidy

@@ -14,6 +14,7 @@ Checks: >-
   portability-*,
   readability-*,
   -modernize-*,
+  -bugprone-easily-swappable-parameters,
   -hicpp-deprecated-headers,
   -hicpp-function-size,
   -hicpp-no-array-decay,
@@ -22,6 +23,7 @@ Checks: >-
   -google-readability-todo,
   -google-runtime-references,
   -google-readability-function-size,
+  -readability-identifier-length,
   -readability-avoid-const-params-in-decls,
   -readability-magic-numbers,
   -readability-function-size,

c/cavl.h

@@ -139,7 +139,7 @@ static inline Cavl* cavlPrivateAdjustBalance(Cavl* const x, const bool increment
         const bool   r    = new_bf < 0;   // bf<0 if left-heavy --> right rotation is needed.
         const int8_t sign = r ? +1 : -1;  // Positive if we are rotating right.
         Cavl* const  z    = x->lr[!r];
-        CAVL_ASSERT(z != NULL);   // Heavy side cannot be empty.
+        CAVL_ASSERT(z != NULL);   // Heavy side cannot be empty.  NOLINTNEXTLINE(clang-analyzer-core.NullDereference)
         if ((z->bf * sign) <= 0)  // Parent and child are heavy on the same side or the child is balanced.
         {
             out = z;

c/test.cpp

@@ -98,8 +98,8 @@ void remove(Node<T>** const root, const Node<T>* const n)
 template <typename T>
 std::uint8_t getHeight(const Node<T>* const n)
 {
-    return (n != nullptr) ? std::uint8_t(1U + std::max(getHeight(reinterpret_cast<Node<T>*>(n->lr[0])),
-                                                       getHeight(reinterpret_cast<Node<T>*>(n->lr[1]))))
+    return (n != nullptr) ? static_cast<std::uint8_t>(1U + std::max(getHeight(reinterpret_cast<Node<T>*>(n->lr[0])),
+                                                                    getHeight(reinterpret_cast<Node<T>*>(n->lr[1]))))
                           : 0;
 }
 
@@ -154,17 +154,21 @@ void printGraphviz(const Node<T>* const nd)
     std::puts("nodesep=0.0;ranksep=0.3;splines=false;");
     traverse<true>(nd, [](const Node<T>* const x) {
         const char* const fill_color = (x->bf == 0) ? "black" : ((x->bf > 0) ? "orange" : "blue");
-        std::printf("%u[fillcolor=%s];", unsigned(x->value), fill_color);
+        std::printf("%u[fillcolor=%s];", static_cast<unsigned>(x->value), fill_color);
     });
     std::puts("");
     traverse<true>(nd, [](const Node<T>* const x) {
         if (x->lr[0] != nullptr)
         {
-            std::printf("%u:sw->%u:n;", unsigned(x->value), unsigned(reinterpret_cast<Node<T>*>(x->lr[0])->value));
+            std::printf("%u:sw->%u:n;",
+                        static_cast<unsigned>(x->value),
+                        static_cast<unsigned>(reinterpret_cast<Node<T>*>(x->lr[0])->value));
         }
         if (x->lr[1] != nullptr)
         {
-            std::printf("%u:se->%u:n;", unsigned(x->value), unsigned(reinterpret_cast<Node<T>*>(x->lr[1])->value));
+            std::printf("%u:se->%u:n;",
+                        static_cast<unsigned>(x->value),
+                        static_cast<unsigned>(reinterpret_cast<Node<T>*>(x->lr[1])->value));
         }
     });
     std::puts("\n}");
@@ -843,8 +847,8 @@ void testRemovalA()
     //  1   3       7
     std::puts("REMOVE 4:");
     remove(&root, &t[4]);
-    TEST_ASSERT_EQUAL(&t[5], root);
     print(root);
+    TEST_ASSERT_EQUAL(&t[5], root);
     TEST_ASSERT_NULL(findBrokenBalanceFactor(root));
     TEST_ASSERT_NULL(findBrokenAncestry(root));
     TEST_ASSERT_EQUAL(6, checkAscension(root));
@@ -1385,12 +1389,14 @@ void testMutationRandomized()
     }
 
     std::puts("Randomized test finished. Final state:");
-    std::printf("\tsize:         %u\n", unsigned(size));
-    std::printf("\tcnt_addition: %u\n", unsigned(cnt_addition));
-    std::printf("\tcnt_removal:  %u\n", unsigned(cnt_removal));
+    std::printf("\tsize:         %u\n", static_cast<unsigned>(size));
+    std::printf("\tcnt_addition: %u\n", static_cast<unsigned>(cnt_addition));
+    std::printf("\tcnt_removal:  %u\n", static_cast<unsigned>(cnt_removal));
     if (root != nullptr)
     {
-        std::printf("\tmin/max:      %u/%u\n", unsigned(root->min()->value), unsigned(root->max()->value));
+        std::printf("\tmin/max:      %u/%u\n",
+                    static_cast<unsigned>(root->min()->value),
+                    static_cast<unsigned>(root->max()->value));
     }
     printGraphviz(root);
     validate();