From dd78b40fbdd1efd8702fd3d4c654bab6e275d022 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Wed, 26 Jun 2024 13:50:49 +0300 Subject: [PATCH] Add support for C++14 --- .github/workflows/main.yml | 20 +++---- .idea/dictionaries/pavel.xml | 2 + CMakeLists.txt | 26 +++++---- c++/cavl.hpp | 36 ++++++++----- c++/test.cpp | 101 +++++++++++++++++++++-------------- c/test.cpp | 2 + 6 files changed, 116 insertions(+), 71 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 6adb92d..32b3dbe 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -10,16 +10,16 @@ jobs: toolchain: ['clang', 'gcc'] include: - toolchain: gcc - c-compiler: gcc-12 - cxx-compiler: g++-12 + c-compiler: gcc-13 + cxx-compiler: g++-13 - toolchain: clang c-compiler: clang cxx-compiler: clang++ steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Install Dependencies - run: sudo apt install gcc-multilib g++-multilib clang-tidy-14 + run: sudo apt install gcc-multilib g++-multilib clang-tidy - name: Configure CMake run: > @@ -46,13 +46,13 @@ jobs: build_type: [Release, MinSizeRel] include: - toolchain: gcc - c-compiler: gcc-12 - cxx-compiler: g++-12 + c-compiler: gcc-13 + cxx-compiler: g++-13 - toolchain: clang c-compiler: clang cxx-compiler: clang++ steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Install Dependencies run: sudo apt install gcc-multilib g++-multilib @@ -78,10 +78,10 @@ jobs: style_check: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 - - uses: DoozyX/clang-format-lint-action@v0.14 + - uses: actions/checkout@v4 + - uses: DoozyX/clang-format-lint-action@v0.18 with: source: '.' exclude: './unity' extensions: 'c,h,cpp,hpp' - clangFormatVersion: 14 + clangFormatVersion: 18 diff --git a/.idea/dictionaries/pavel.xml b/.idea/dictionaries/pavel.xml index 2241aff..8059d9c 100644 --- a/.idea/dictionaries/pavel.xml +++ b/.idea/dictionaries/pavel.xml @@ -8,6 +8,8 @@ miscompiles nihil nodesep + nolintbegin + nolintend noninfringement penwidth ranksep diff --git a/CMakeLists.txt b/CMakeLists.txt index 0c05d4f..984dbc8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -10,7 +10,7 @@ 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-14 clang-tidy) + find_program(clang_tidy NAMES clang-tidy) if (NOT clang_tidy) message(FATAL_ERROR "Could not locate clang-tidy") endif () @@ -18,14 +18,14 @@ if (NOT NO_STATIC_ANALYSIS) set(CMAKE_CXX_CLANG_TIDY ${clang_tidy}) endif () -find_program(clang_format NAMES clang-format-14 clang-format) +find_program(clang_format NAMES clang-format) if (NOT clang_format) message(STATUS "Could not locate clang-format") else () file(GLOB format_files - ${CMAKE_CURRENT_SOURCE_DIR}/c/*.[ch]pp - ${CMAKE_CURRENT_SOURCE_DIR}/c/*.[ch] - ${CMAKE_CURRENT_SOURCE_DIR}/c++/*.[ch]pp + ${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}) @@ -33,7 +33,7 @@ endif () set(CMAKE_C_STANDARD 11) set(CMAKE_C_EXTENSIONS OFF) -set(CMAKE_CXX_STANDARD 17) +set(CMAKE_CXX_STANDARD 23) set(CMAKE_CXX_EXTENSIONS OFF) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Werror -pedantic -fstrict-aliasing") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wdouble-promotion -Wswitch-enum -Wfloat-equal -Wundef") @@ -53,10 +53,16 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU") # 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() +endif () target_link_libraries(test_c unity) add_test("run_test_c" "test_c") -add_executable(test_cpp ${CMAKE_CURRENT_SOURCE_DIR}/c++/test.cpp) -target_link_libraries(test_cpp unity) -add_test("run_test_cpp" "test_cpp") +add_executable(test_cpp23 ${CMAKE_CURRENT_SOURCE_DIR}/c++/test.cpp) +set_target_properties(test_cpp23 PROPERTIES CXX_STANDARD 23) +target_link_libraries(test_cpp23 unity) +add_test("run_test_cpp23" "test_cpp23") + +add_executable(test_cpp14 ${CMAKE_CURRENT_SOURCE_DIR}/c++/test.cpp) +set_target_properties(test_cpp14 PROPERTIES CXX_STANDARD 14) +target_link_libraries(test_cpp14 unity) +add_test("run_test_cpp14" "test_cpp14") diff --git a/c++/cavl.hpp b/c++/cavl.hpp index f324bdb..b386807 100644 --- a/c++/cavl.hpp +++ b/c++/cavl.hpp @@ -51,6 +51,15 @@ class Tree; template class Node { + // Polyfill for C++17's std::invoke_result_t. + template + using invoke_result = +#if __cplusplus >= 201703L + std::invoke_result_t; +#else + std::result_of_t; +#endif + public: /// Helper aliases. using TreeType = Tree; @@ -143,9 +152,10 @@ class Node /// traversal will stop when the first true value is returned, which is propagated back to the caller; if none /// of the calls returned true or the tree is empty, a default value is constructed and returned. /// The tree shall not be modified while traversal is in progress, otherwise bad memory access will likely occur. - template > - static auto traverse(Derived* const root, const Vis& visitor, const bool reverse = false) - -> std::enable_if_t, R> + template > + static auto traverse(Derived* const root, + const Vis& visitor, + const bool reverse = false) -> std::enable_if_t::value, R> { if (Node* const n = root) { @@ -163,7 +173,7 @@ class Node } template static auto traverse(Derived* const root, const Vis& visitor, const bool reverse = false) - -> std::enable_if_t>> + -> std::enable_if_t>::value> { if (Node* const n = root) { @@ -172,9 +182,10 @@ class Node Node::traverse(down(n->lr[!reverse]), visitor, reverse); } } - template > - static auto traverse(const Derived* const root, const Vis& visitor, const bool reverse = false) - -> std::enable_if_t, R> + template > + static auto traverse(const Derived* const root, + const Vis& visitor, + const bool reverse = false) -> std::enable_if_t::value, R> { if (const Node* const n = root) { @@ -192,7 +203,7 @@ class Node } template static auto traverse(const Derived* const root, const Vis& visitor, const bool reverse = false) - -> std::enable_if_t>> + -> std::enable_if_t>::value> { if (const Node* const n = root) { @@ -565,13 +576,13 @@ class Tree final template auto traverse(const Vis& visitor, const bool reverse = false) { - TraversalIndicatorUpdater upd(*this); + const TraversalIndicatorUpdater upd(*this); return NodeType::template traverse(*this, visitor, reverse); } template auto traverse(const Vis& visitor, const bool reverse = false) const { - TraversalIndicatorUpdater upd(*this); + const TraversalIndicatorUpdater upd(*this); return NodeType::template traverse(*this, visitor, reverse); } @@ -603,8 +614,9 @@ class Tree final auto empty() const noexcept { return root_ == nullptr; } private: - static_assert(!std::is_polymorphic_v); - static_assert(std::is_same_v, typename NodeType::TreeType>); + static_assert(!std::is_polymorphic::value, + "Internal check: The node type must not be a polymorphic type"); + static_assert(std::is_same, typename NodeType::TreeType>::value, "Internal check: Bad type alias"); /// We use a simple boolean flag instead of a nesting counter to avoid race conditions on the counter update. /// This implies that in the case of concurrent or recursive traversal (more than one call to traverse() within diff --git a/c++/test.cpp b/c++/test.cpp index 429cf4f..7c87c6f 100644 --- a/c++/test.cpp +++ b/c++/test.cpp @@ -4,17 +4,28 @@ #include #include #include +#include #include #include #include -#include +#include #include #include #include #include +#include #include +#include #include +#if __cplusplus >= 201703L +# define NODISCARD [[nodiscard]] +# define UNUSED [[maybe_unused]] +#else +# define NODISCARD +# define UNUSED +#endif + void setUp() {} void tearDown() {} @@ -40,7 +51,7 @@ class My : public cavl::Node using Self::min; using Self::max; - [[nodiscard]] auto getValue() const -> std::uint16_t { return value; } + NODISCARD auto getValue() const -> std::uint16_t { return value; } private: std::uint16_t value = 0; @@ -49,23 +60,23 @@ class My : public cavl::Node // defined in the derived class. That would trigger compilation error in this case, but may be deadly in the field. using E = struct {}; - [[maybe_unused]] E up; - [[maybe_unused]] E lr; - [[maybe_unused]] E bf; + UNUSED E up; // NOLINT(*-unused-private-field) + UNUSED E lr; // NOLINT(*-unused-private-field) + UNUSED E bf; // NOLINT(*-unused-private-field) }; using MyTree = cavl::Tree; -static_assert(std::is_same_v); -static_assert(std::is_same_v, MyTree::NodeType>); +static_assert(std::is_same::value, ""); +static_assert(std::is_same, MyTree::NodeType>::value, ""); template using N = typename cavl::Node::DerivedType; -static_assert(std::is_same_v>); +static_assert(std::is_same>::value, ""); template -[[nodiscard]] bool checkLinkage(const N* const self, - const N* const up, - const std::array*, 2>& lr, - const std::int8_t bf) +NODISCARD bool checkLinkage(const N* const self, + const N* const up, + const std::array*, 2>& lr, + const std::int8_t bf) { return (self->getParentNode() == up) && // (self->getChildNode(false) == lr.at(0)) && (self->getChildNode(true) == lr.at(1)) && // @@ -76,15 +87,16 @@ template } template -[[nodiscard]] auto getHeight(const N* const n) -> std::int8_t +NODISCARD auto getHeight(const N* const n) -> std::int8_t { return (n != nullptr) ? static_cast(1 + std::max(getHeight(n->getChildNode(false)), // getHeight(n->getChildNode(true)))) : 0; } +/// Returns the size if the tree is ordered correctly, otherwise SIZE_MAX. template -[[nodiscard]] std::optional checkOrdering(const N* const root) +NODISCARD std::size_t checkOrdering(const N* const root) { const N* prev = nullptr; bool valid = true; @@ -97,15 +109,15 @@ template prev = &nd; size++; }); - return valid ? std::optional(size) : std::optional{}; + return valid ? size : std::numeric_limits::max(); } template -[[nodiscard]] const N* findBrokenAncestry(const N* const n, const N* const parent = nullptr) +NODISCARD const N* findBrokenAncestry(const N* const n, const N* const parent = nullptr) { if ((n != nullptr) && (n->getParentNode() == parent)) { - for (bool v : {true, false}) + for (const bool v : {true, false}) { if (const N* p = findBrokenAncestry(n->getChildNode(v), n)) { @@ -118,7 +130,7 @@ template } template -[[nodiscard]] const N* findBrokenBalanceFactor(const N* const n) +NODISCARD const N* findBrokenBalanceFactor(const N* const n) { if (n != nullptr) { @@ -130,7 +142,7 @@ template { return n; } - for (bool v : {true, false}) + for (const bool v : {true, false}) { if (auto* const ch = n->getChildNode(v)) { @@ -145,7 +157,7 @@ template } template -[[nodiscard]] auto toGraphviz(const cavl::Tree& tr) -> std::string +NODISCARD auto toGraphviz(const cavl::Tree& tr) -> std::string { std::ostringstream ss; ss << "// Feed the following text to Graphviz, or use an online UI like https://edotor.net/\n" @@ -154,7 +166,7 @@ template << "edge[arrowhead=none,penwidth=2];\n" << "nodesep=0.0;ranksep=0.3;splines=false;\n"; tr.traverse([&](const typename cavl::Tree::DerivedType& x) { - const char* const fill_color = + const char* const fill_color = // NOLINTNEXTLINE(*-avoid-nested-conditional-operator) (x.getBalanceFactor() == 0) ? "black" : ((x.getBalanceFactor() > 0) ? "orange" : "blue"); ss << x.getValue() << "[fillcolor=" << fill_color << "];"; }); @@ -212,7 +224,7 @@ void testManual(const std::function& factory) TEST_ASSERT(!tr.empty()); TEST_ASSERT_NULL(findBrokenBalanceFactor(tr)); TEST_ASSERT_NULL(findBrokenAncestry(tr)); - TEST_ASSERT_TRUE(checkOrdering(tr)); + TEST_ASSERT_TRUE(checkOrdering(tr) < std::numeric_limits::max()); }; // Insert out of order to cover more branches in the insertion method. // We can't really go full random because we need perfectly balanced tree for the manual tests that follow. @@ -798,25 +810,25 @@ class V : public cavl::Node V& operator=(const V&) = delete; V& operator=(V&&) = delete; - [[nodiscard]] virtual auto getValue() const -> std::uint16_t = 0; + NODISCARD virtual auto getValue() const -> std::uint16_t = 0; private: using E = struct {}; - [[maybe_unused]] E up; - [[maybe_unused]] E lr; - [[maybe_unused]] E bf; + UNUSED E up; // NOLINT(*-unused-private-field) + UNUSED E lr; // NOLINT(*-unused-private-field) + UNUSED E bf; // NOLINT(*-unused-private-field) }; using VTree = cavl::Tree; -static_assert(std::is_same_v); -static_assert(std::is_same_v, VTree::NodeType>); +static_assert(std::is_same::value, ""); +static_assert(std::is_same, VTree::NodeType>::value, ""); // Dummy polymorphism for testing purposes. template class VValue : public VValue(Value - 1)> { public: - [[nodiscard]] auto getValue() const -> std::uint16_t override + NODISCARD auto getValue() const -> std::uint16_t override { return static_cast(VValue(Value - 1)>::getValue() + 1); } @@ -825,24 +837,33 @@ template <> class VValue<0> : public V { public: - [[nodiscard]] auto getValue() const -> std::uint16_t override { return 0; } + NODISCARD auto getValue() const -> std::uint16_t override { return 0; } }; -template -auto makeV(const std::uint8_t val) -> V* +template = Limit), int> = 0> +auto makeV_impl(const std::uint8_t val) -> V* { if (val == Candidate) { - return new VValue(); // NOLINT - } - if constexpr (Candidate < std::numeric_limits::max()) - { - return makeV(val); + return new VValue(); // NOLINT(*-owning-memory) } - else + return nullptr; +} + +template = 0> +auto makeV_impl(const std::uint8_t val) -> V* +{ + if (val == Candidate) { - return nullptr; + return new VValue(); // NOLINT(*-owning-memory) } + return makeV_impl(val); +} + +template +auto makeV(const std::uint8_t val) -> V* +{ + return makeV_impl::max()>(val); } void testManualV() @@ -857,9 +878,11 @@ int main(const int argc, const char* const argv[]) const auto seed = static_cast((argc > 1) ? std::atoll(argv[1]) : std::time(nullptr)); // NOLINT std::cout << "Randomness seed: " << seed << std::endl; std::srand(seed); + // NOLINTBEGIN(misc-include-cleaner) UNITY_BEGIN(); RUN_TEST(testManualMy); RUN_TEST(testManualV); RUN_TEST(testRandomized); return UNITY_END(); + // NOLINTEND(misc-include-cleaner) } diff --git a/c/test.cpp b/c/test.cpp index b4c7071..57e687e 100644 --- a/c/test.cpp +++ b/c/test.cpp @@ -1409,6 +1409,7 @@ int main(const int argc, const char* const argv[]) const auto seed = static_cast((argc > 1) ? std::atoll(argv[1]) : std::time(nullptr)); // NOLINT std::printf("Randomness seed: %u\n", seed); std::srand(seed); + // NOLINTBEGIN(misc-include-cleaner) UNITY_BEGIN(); RUN_TEST(testCheckAscension); RUN_TEST(testRotation); @@ -1421,4 +1422,5 @@ int main(const int argc, const char* const argv[]) RUN_TEST(testMutationManual); RUN_TEST(testMutationRandomized); return UNITY_END(); + // NOLINTEND(misc-include-cleaner) }