From c7cd4dcc1a7babf5cdddac45770a636bc195aa1d Mon Sep 17 00:00:00 2001 From: Geoffrey M Gunter Date: Sun, 4 Aug 2024 00:45:07 -0700 Subject: [PATCH] Add compiler warning flags --- cmake/WhirlwindWarnings.cmake | 71 +++++++++++ .../whirlwind/graph/shortest_path_forest.hpp | 12 +- test/CMakeLists.txt | 6 +- test/graph/csr_graph.cpp | 110 +++++++++--------- test/graph/forest.cpp | 46 ++++---- test/math/test_numbers.cpp | 4 +- 6 files changed, 162 insertions(+), 87 deletions(-) create mode 100644 cmake/WhirlwindWarnings.cmake diff --git a/cmake/WhirlwindWarnings.cmake b/cmake/WhirlwindWarnings.cmake new file mode 100644 index 0000000..361ef87 --- /dev/null +++ b/cmake/WhirlwindWarnings.cmake @@ -0,0 +1,71 @@ +include_guard(GLOBAL) + +set(WHIRLWIND_CXX_WARNINGS) +if(CMAKE_CXX_COMPILER_ID STREQUAL GNU) + set(WHIRLWIND_CXX_WARNINGS + # cmake-format: sortable + -Wall + -Wcast-align + -Wcast-qual + -Wconversion + -Wdouble-promotion + -Wduplicated-branches + -Werror + -Wextra + -Wlogical-op + -Wnon-virtual-dtor + -Wold-style-cast + -Wpedantic + -Wshadow + -Wsign-conversion + -Wundef + ) +elseif(CMAKE_CXX_COMPILER_ID MATCHES "^(Apple)?Clang$") + set(WHIRLWIND_CXX_WARNINGS + # cmake-format: sortable + -Wall + -Wbad-function-cast + -Wcast-function-type + -Wcast-qual + -Wconditional-uninitialized + -Wconversion + -Wcuda-compat + -Wdeprecated + -Wdouble-promotion + -Wduplicate-decl-specifier + -Wduplicate-method-arg + -Wduplicate-method-match + -Werror + -Wextra + -Wextra-semi + -Wheader-hygiene + -Wimplicit + -Winconsistent-missing-destructor-override + -Wloop-analysis + -Wnarrowing + -Wnon-virtual-dtor + -Wold-style-cast + -Woverriding-method-mismatch + -Wpedantic + -Wpointer-arith + -Wshadow + -Wstatic-in-inline + -Wswitch-enum + -Wundefined-reinterpret-cast + -Wunreachable-code-aggressive + -Wunused-member-function + -Wunused-template + -Wvector-conversion + ) +elseif(CMAKE_CXX_COMPILER_ID STREQUAL MSVC) + set(WHIRLWIND_CXX_WARNINGS /W3 /WX) +endif() + +# Define an interface-only target with a list of desired C++ compiler warning flags +# enabled. The flags can then be applied to any compiled target by linking with the +# interface target via `target_link_libraries()`. +add_library(whirlwind-warnings INTERFACE) +add_library(whirlwind::warnings ALIAS whirlwind-warnings) +target_compile_options( + whirlwind-warnings INTERFACE $<$:${WHIRLWIND_CXX_WARNINGS}> +) diff --git a/include/whirlwind/graph/shortest_path_forest.hpp b/include/whirlwind/graph/shortest_path_forest.hpp index a698b13..f2c0e01 100644 --- a/include/whirlwind/graph/shortest_path_forest.hpp +++ b/include/whirlwind/graph/shortest_path_forest.hpp @@ -43,13 +43,13 @@ class ShortestPathForest : public Base { using base_type::graph; - explicit constexpr ShortestPathForest(const graph_type& graph) - : base_type(graph), - label_(graph.num_vertices(), label_type::unreached), - distance_(graph.num_vertices(), infinity()) + explicit constexpr ShortestPathForest(const graph_type& g) + : base_type(g), + label_(g.num_vertices(), label_type::unreached), + distance_(g.num_vertices(), infinity()) { - WHIRLWIND_DEBUG_ASSERT(std::size(label_) == this->graph().num_vertices()); - WHIRLWIND_DEBUG_ASSERT(std::size(distance_) == this->graph().num_vertices()); + WHIRLWIND_DEBUG_ASSERT(std::size(label_) == g.num_vertices()); + WHIRLWIND_DEBUG_ASSERT(std::size(distance_) == g.num_vertices()); } [[nodiscard]] constexpr auto diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 8eb601e..1277e8a 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -1,5 +1,8 @@ find_package(Catch2 3 CONFIG REQUIRED) +list(APPEND CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake) +include(WhirlwindWarnings) + # Add test executable. add_executable( test-whirlwind # cmake-format: sortable @@ -12,7 +15,8 @@ add_executable( math/test_numbers.cpp ) target_link_libraries( - test-whirlwind PRIVATE whirlwind::whirlwind Catch2::Catch2WithMain + test-whirlwind PRIVATE Catch2::Catch2WithMain whirlwind::warnings + whirlwind::whirlwind ) set_target_properties(test-whirlwind PROPERTIES CXX_SCAN_FOR_MODULES OFF) diff --git a/test/graph/csr_graph.cpp b/test/graph/csr_graph.cpp index c79937c..388db38 100644 --- a/test/graph/csr_graph.cpp +++ b/test/graph/csr_graph.cpp @@ -17,30 +17,30 @@ TEST_CASE("CSRGraph (empty)", "[graph]") SECTION("num_{vertices,edges}") { - CHECK(graph.num_vertices() == 0); - CHECK(graph.num_edges() == 0); + CHECK(graph.num_vertices() == 0U); + CHECK(graph.num_edges() == 0U); } SECTION("contains_{vertex,edge}") { - CHECK(!graph.contains_vertex(0)); - CHECK(!graph.contains_edge(0)); + CHECK(!graph.contains_vertex(0U)); + CHECK(!graph.contains_edge(0U)); } } TEST_CASE("CSRGraph", "[graph]") { auto edgelist = ww::EdgeList(); - edgelist.add_edge(0, 1); - edgelist.add_edge(0, 2); - edgelist.add_edge(0, 3); - edgelist.add_edge(2, 1); - edgelist.add_edge(3, 0); + edgelist.add_edge(0U, 1U); + edgelist.add_edge(0U, 2U); + edgelist.add_edge(0U, 3U); + edgelist.add_edge(2U, 1U); + edgelist.add_edge(3U, 0U); const auto graph = ww::CSRGraph(edgelist); - const auto vertices = {0, 1, 2, 3}; - const auto edges = {0, 1, 2, 3, 4}; + const auto vertices = {0U, 1U, 2U, 3U}; + const auto edges = {0U, 1U, 2U, 3U, 4U}; SECTION("{vertex,edge,size}_type") { @@ -56,8 +56,8 @@ TEST_CASE("CSRGraph", "[graph]") SECTION("num_{vertices,edges}") { - CHECK(graph.num_vertices() == 4); - CHECK(graph.num_edges() == 5); + CHECK(graph.num_vertices() == 4U); + CHECK(graph.num_edges() == 5U); } SECTION("get_{vertex,edge}_id") @@ -78,47 +78,47 @@ TEST_CASE("CSRGraph", "[graph]") SECTION("contains_{vertex,edge}") { - CHECK(graph.contains_vertex(0)); - CHECK(graph.contains_vertex(3)); - CHECK(!graph.contains_vertex(-1)); - CHECK(!graph.contains_vertex(4)); + CHECK(graph.contains_vertex(0U)); + CHECK(graph.contains_vertex(3U)); + CHECK(!graph.contains_vertex(999U)); + CHECK(!graph.contains_vertex(4U)); - CHECK(graph.contains_edge(0)); - CHECK(graph.contains_edge(4)); - CHECK(!graph.contains_edge(-1)); - CHECK(!graph.contains_edge(5)); + CHECK(graph.contains_edge(0U)); + CHECK(graph.contains_edge(4U)); + CHECK(!graph.contains_edge(999U)); + CHECK(!graph.contains_edge(5U)); } SECTION("outdegree") { - CHECK(graph.outdegree(0) == 3); - CHECK(graph.outdegree(1) == 0); - CHECK(graph.outdegree(2) == 1); - CHECK(graph.outdegree(3) == 1); + CHECK(graph.outdegree(0U) == 3U); + CHECK(graph.outdegree(1U) == 0U); + CHECK(graph.outdegree(2U) == 1U); + CHECK(graph.outdegree(3U) == 1U); } } TEST_CASE("CSRGraph (nonconsecutive vertices)", "[graph]") { auto edgelist = ww::EdgeList(); - edgelist.add_edge(0, 1); - edgelist.add_edge(1, 2); - edgelist.add_edge(4, 5); + edgelist.add_edge(0U, 1U); + edgelist.add_edge(1U, 2U); + edgelist.add_edge(4U, 5U); const auto graph = ww::CSRGraph(edgelist); SECTION("num_{vertices,edges}") { - CHECK(graph.num_vertices() == 6); - CHECK(graph.num_edges() == 3); + CHECK(graph.num_vertices() == 6U); + CHECK(graph.num_edges() == 3U); } SECTION("{vertices,edges}") { - const auto vertices = {0, 1, 2, 3, 4, 5}; + const auto vertices = {0U, 1U, 2U, 3U, 4U, 5U}; CHECK(ranges::equal(graph.vertices(), vertices)); - const auto edges = {0, 1, 2}; + const auto edges = {0U, 1U, 2U}; CHECK(ranges::equal(graph.edges(), edges)); } @@ -134,26 +134,26 @@ TEST_CASE("CSRGraph (nonconsecutive vertices)", "[graph]") TEST_CASE("CSRGraph (unsorted edges)", "[graph]") { auto edgelist = ww::EdgeList(); - edgelist.add_edge(0, 3); - edgelist.add_edge(2, 1); - edgelist.add_edge(0, 2); - edgelist.add_edge(3, 0); - edgelist.add_edge(0, 1); + edgelist.add_edge(0U, 3U); + edgelist.add_edge(2U, 1U); + edgelist.add_edge(0U, 2U); + edgelist.add_edge(3U, 0U); + edgelist.add_edge(0U, 1U); const auto graph = ww::CSRGraph(edgelist); SECTION("num_{vertices,edges}") { - CHECK(graph.num_vertices() == 4); - CHECK(graph.num_edges() == 5); + CHECK(graph.num_vertices() == 4U); + CHECK(graph.num_edges() == 5U); } SECTION("{vertices,edges}") { - const auto vertices = {0, 1, 2, 3}; + const auto vertices = {0U, 1U, 2U, 3U}; CHECK(ranges::equal(graph.vertices(), vertices)); - const auto edges = {0, 1, 2, 3, 4}; + const auto edges = {0U, 1U, 2U, 3U, 4U}; CHECK(ranges::equal(graph.edges(), edges)); } } @@ -161,43 +161,43 @@ TEST_CASE("CSRGraph (unsorted edges)", "[graph]") TEST_CASE("CSRGraph (parallel edges)", "[graph]") { auto edgelist = ww::EdgeList(); - edgelist.add_edge(0, 1); - edgelist.add_edge(0, 1); + edgelist.add_edge(0U, 1U); + edgelist.add_edge(0U, 1U); const auto graph = ww::CSRGraph(edgelist); SECTION("num_{vertices,edges}") { - CHECK(graph.num_vertices() == 2); - CHECK(graph.num_edges() == 2); + CHECK(graph.num_vertices() == 2U); + CHECK(graph.num_edges() == 2U); } - SECTION("outdegree") { CHECK(graph.outdegree(0) == 2); } + SECTION("outdegree") { CHECK(graph.outdegree(0) == 2U); } } TEST_CASE("CSRGraph (self loops)", "[graph]") { auto edgelist = ww::EdgeList(); - edgelist.add_edge(1, 0); - edgelist.add_edge(1, 1); - edgelist.add_edge(1, 1); - edgelist.add_edge(1, 2); + edgelist.add_edge(1U, 0U); + edgelist.add_edge(1U, 1U); + edgelist.add_edge(1U, 1U); + edgelist.add_edge(1U, 2U); const auto graph = ww::CSRGraph(edgelist); SECTION("num_{vertices,edges}") { - CHECK(graph.num_vertices() == 3); - CHECK(graph.num_edges() == 4); + CHECK(graph.num_vertices() == 3U); + CHECK(graph.num_edges() == 4U); } SECTION("contains_vertex") { - CHECK(graph.contains_vertex(0)); - CHECK(graph.contains_vertex(2)); + CHECK(graph.contains_vertex(0U)); + CHECK(graph.contains_vertex(2U)); } - SECTION("outdegree") { CHECK(graph.outdegree(1) == 4); } + SECTION("outdegree") { CHECK(graph.outdegree(1U) == 4U); } } } // namespace diff --git a/test/graph/forest.cpp b/test/graph/forest.cpp index ae3a0ab..b859928 100644 --- a/test/graph/forest.cpp +++ b/test/graph/forest.cpp @@ -17,7 +17,7 @@ namespace ww = whirlwind; TEST_CASE("Forest (const)", "[graph]") { - const auto graph = ww::RectangularGridGraph<>(4, 4); + const auto graph = ww::RectangularGridGraph<>(4U, 4U); const auto forest = ww::Forest(graph); SECTION("{graph,vertex,edge,pred}_type") @@ -56,8 +56,8 @@ TEST_CASE("Forest (const)", "[graph]") TEST_CASE("Forest (non-const)", "[graph]") { auto edgelist = ww::EdgeList(); - edgelist.add_edge(1, 2); - edgelist.add_edge(2, 3); + edgelist.add_edge(1U, 2U); + edgelist.add_edge(2U, 3U); const auto graph = ww::CSRGraph(edgelist); @@ -65,35 +65,35 @@ TEST_CASE("Forest (non-const)", "[graph]") SECTION("set_predecessor") { - CHECK(forest.predecessor_vertex(2) == 2); - forest.set_predecessor(2, 1, 0); - CHECK(forest.predecessor_vertex(2) == 1); - CHECK(forest.predecessor_edge(2) == 0); - - CHECK(forest.predecessor_vertex(3) == 3); - forest.set_predecessor(3, {2, 1}); - CHECK(forest.predecessor_vertex(3) == 2); - CHECK(forest.predecessor_edge(3) == 1); + CHECK(forest.predecessor_vertex(2U) == 2U); + forest.set_predecessor(2U, 1U, 0U); + CHECK(forest.predecessor_vertex(2U) == 1U); + CHECK(forest.predecessor_edge(2U) == 0U); + + CHECK(forest.predecessor_vertex(3U) == 3U); + forest.set_predecessor(3U, {2U, 1U}); + CHECK(forest.predecessor_vertex(3U) == 2U); + CHECK(forest.predecessor_edge(3U) == 1U); } SECTION("make_root_vertex") { - CHECK(forest.is_root_vertex(2)); - forest.set_predecessor(2, 1, 0); - CHECK(!forest.is_root_vertex(2)); - forest.make_root_vertex(2); - CHECK(forest.is_root_vertex(2)); + CHECK(forest.is_root_vertex(2U)); + forest.set_predecessor(2U, 1U, 0U); + CHECK(!forest.is_root_vertex(2U)); + forest.make_root_vertex(2U); + CHECK(forest.is_root_vertex(2U)); } SECTION("reset") { - forest.set_predecessor(2, 1, 0); - forest.set_predecessor(3, 2, 1); - CHECK(!forest.is_root_vertex(2)); - CHECK(!forest.is_root_vertex(3)); + forest.set_predecessor(2U, 1U, 0U); + forest.set_predecessor(3U, 2U, 1U); + CHECK(!forest.is_root_vertex(2U)); + CHECK(!forest.is_root_vertex(3U)); forest.reset(); - CHECK(forest.is_root_vertex(2)); - CHECK(forest.is_root_vertex(3)); + CHECK(forest.is_root_vertex(2U)); + CHECK(forest.is_root_vertex(3U)); } } diff --git a/test/math/test_numbers.cpp b/test/math/test_numbers.cpp index 09edea8..f54fbb8 100644 --- a/test/math/test_numbers.cpp +++ b/test/math/test_numbers.cpp @@ -32,7 +32,7 @@ TEST_CASE("one", "[numbers]") TEST_CASE("pi", "[numbers]") { using Catch::Matchers::WithinAbs; - CHECK_THAT(ww::pi(), WithinAbs(3.141'592'7f, 1e-7)); + CHECK_THAT(ww::pi(), WithinAbs(3.141'592'7, 1e-7)); CHECK_THAT(ww::pi(), WithinAbs(3.141'592'653'589'793'2, 1e-16)); } @@ -45,7 +45,7 @@ TEST_CASE("pi (consteval)", "[numbers]") TEST_CASE("tau", "[numbers]") { using Catch::Matchers::WithinAbs; - CHECK_THAT(ww::tau(), WithinAbs(6.283'185'3f, 2e-7)); + CHECK_THAT(ww::tau(), WithinAbs(6.283'185'3, 2e-7)); CHECK_THAT(ww::tau(), WithinAbs(6.283'185'307'179'586'4, 1e-16)); }