Compatability & Warnings as Errors

CMakeLists.txt

@@ -9,13 +9,9 @@ include(GNUInstallDirs)
 set(CMAKE_CXX_STANDARD 17)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 
-
-# Configure the option whether you want to use the systems own allocater
-# DHB_WITH_SYSTEM_ALLOCATOR=ON
-# or our own written allocator strategy
-# DHB_WITH_SYSTEM_ALLOCATOR=OFF
 option(DHB_WITH_SYSTEM_ALLOCATOR "Use DHB System Allocator" OFF)
 option(DHB_WITH_64BIT_IDS "Use 64 bit IDs." OFF)
+option(DHB_WARNINGS_AS_ERRORS "Sets warning level high, treats warnings as errors ." OFF)
 
 add_library(dhb STATIC)
 
@@ -42,18 +38,21 @@ target_sources(${PROJECT_NAME}
 # Options check and set definitions
 # ===================================
 if (DHB_WITH_SYSTEM_ALLOCATOR)
-  MESSAGE(STATUS "Using DHB System Allocator.")
+  MESSAGE(STATUS "DHB uses system allocator.")
   target_compile_definitions(dhb PUBLIC -DDHB_SYSTEM_ALLOCATOR)
 endif()
 
 if (DHB_WITH_64BIT_IDS)
-  MESSAGE(STATUS "Using 64 bit IDs.")
+  MESSAGE(STATUS "DHB uses 64 bit vertex IDs.")
   target_compile_definitions(dhb PUBLIC -DDHB_64BIT_IDS)
 else()
-  MESSAGE(STATUS "Using 32 bit IDs.")
+  MESSAGE(STATUS "DHB uses 32 bit vertex IDs.")
 endif()
 
-set_property(TARGET dhb PROPERTY POSITION_INDEPENDENT_CODE ON)
+if (DHB_WARNINGS_AS_ERRORS)
+  MESSAGE(STATUS "DHB is extra pedantic, throws errors on warnings.")
+  target_compile_options(dhb PRIVATE -Werror -Wall -Wextra -Wpedantic -Wmaybe-uninitialized)
+endif()
 
 find_package(OpenMP)
 # Thanks to NetworKit for the following OpenMP code looking or OpenMP in the
@@ -95,9 +94,6 @@ if(NOT OpenMP_FOUND)
 endif()
 target_link_libraries(${PROJECT_NAME} PUBLIC OpenMP::OpenMP_CXX)
 
-# Be extra pedantic about warnings!
-target_compile_options(${PROJECT_NAME} PRIVATE -Wall -Wextra -Wpedantic -Werror)
-
 target_include_directories(${PROJECT_NAME}
   PRIVATE
     ${CMAKE_CURRENT_SOURCE_DIR}/src
@@ -128,8 +124,10 @@ set_target_properties(${PROJECT_NAME}::${PROJECT_NAME} PROPERTIES
   INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include"
 )
 
-set_target_properties(${PROJECT_NAME} PROPERTIES PUBLIC_HEADER "${public_headers}")
-set_target_properties(${PROJECT_NAME} PROPERTIES DEBUG_POSTFIX "d")
+set_target_properties(dhb PROPERTIES CXX_STANDARD 17)
+set_target_properties(dhb PROPERTIES POSITION_INDEPENDENT_CODE ON)
+set_target_properties(dhb PROPERTIES DEBUG_POSTFIX "d")
+set_target_properties(dhb PROPERTIES PUBLIC_HEADER "${public_headers}")
 
 # ===================================
 # Make a Configuration Package
@@ -192,11 +190,16 @@ if (DHB_BUILD_TESTS)
     test/graph_io.h
     test/graph_io.cpp
     test/matrix_test.cpp
-    test/vec.cpp
+    test/vec_test.cpp
     test/block.cpp
-    test/submatrix.cpp
+    test/submatrix_test.cpp
   )
 
+  if (DHB_WARNINGS_AS_ERRORS)
+    MESSAGE(STATUS "DHB test is extra pedantic, throws errors on warnings.")
+    target_compile_options(dhb_test PRIVATE -Werror -Wall -Wextra -Wpedantic -Wmaybe-uninitialized)
+  endif()
+
   target_link_libraries(dhb_test PRIVATE dhb Catch2::Catch2WithMain OpenMP::OpenMP_CXX)
 
   install(

include/dhb/batcher.h

@@ -69,7 +69,7 @@ template <typename T> class BatchParallelizer {
   public:
     template <typename Iterator, typename GetSourceF, typename Cmp, typename F>
     void operator()(Iterator begin, Iterator end, GetSourceF&& get_source_f, Cmp cmp, F func) {
-        int const t_count = omp_get_max_threads();
+        uint32_t const t_count = omp_get_max_threads();
         size_t const n = end - begin;
         if (t_count == 1 || n < t_count) {
             for (auto it = begin; it != end; ++it)
@@ -90,7 +90,7 @@ template <typename T> class BatchParallelizer {
 
     template <typename Iterator, typename K, typename F>
     void apply(Iterator begin, Iterator end, K key, F func) {
-        size_t const t_count = omp_get_max_threads();
+        uint32_t const t_count = omp_get_max_threads();
         size_t const n = end - begin;
         if (t_count == 1 || n < t_count) {
             for (auto it = begin; it != end; ++it)
@@ -104,7 +104,7 @@ template <typename T> class BatchParallelizer {
 #pragma omp parallel num_threads(t_count)
         {
             size_t const t = omp_get_thread_num();
-            assert(omp_get_num_threads() == t_count);
+            assert(uint32_t(omp_get_num_threads()) == t_count);
 
             auto counts_of_thread = [&](int ct) -> uint64_t* {
                 return &m_batch_counts[ct * (t_count + 1)];
@@ -156,7 +156,6 @@ template <typename T> class BatchParallelizer {
                 auto j_end = ot_counts[t + 1];
                 for (size_t j = j_begin; j < j_end; ++j) {
                     auto i = m_batch_slots[j];
-                    auto edge = *(begin + i);
                     func(*(begin + i));
                 }
                 local_count += j_end - j_begin;
@@ -165,7 +164,7 @@ template <typename T> class BatchParallelizer {
     }
 
     template <typename Iterator, typename K> void distribute(Iterator begin, Iterator end, K key) {
-        int const t_count = omp_get_num_threads();
+        uint32_t const t_count = omp_get_num_threads();
         size_t const n = end - begin;
         if (t_count == 1 || n < t_count) {
             return;
@@ -177,7 +176,7 @@ template <typename T> class BatchParallelizer {
             m_batch_slots.resize(n);
         }
 
-        auto t = omp_get_thread_num();
+        uint32_t t = omp_get_thread_num();
 
         auto counts_of_thread = [&](int ct) -> uint64_t* {
             return &m_batch_counts[ct * (t_count + 1)];
@@ -192,7 +191,7 @@ template <typename T> class BatchParallelizer {
         // First, perform a local counting sort to sort updates according to associated threads.
 
         auto t_counts = counts_of_thread(t);
-        for (int at = 0; at < t_count; ++at)
+        for (uint32_t at = 0; at < t_count; ++at)
             t_counts[at] = 0;
 
         for (size_t i = i_begin; i < i_end; ++i) {
@@ -202,7 +201,7 @@ template <typename T> class BatchParallelizer {
         }
 
         uint64_t psum = 0;
-        for (int at = 0; at < t_count; ++at) {
+        for (uint32_t at = 0; at < t_count; ++at) {
             psum += t_counts[at];
             t_counts[at] = i_begin + psum;
         }
@@ -223,7 +222,7 @@ template <typename T> class BatchParallelizer {
     }
 
     template <typename Iterator, typename F> void map(Iterator begin, Iterator end, F func) {
-        int const t_count = omp_get_num_threads();
+        uint32_t const t_count = omp_get_num_threads();
         size_t const n = end - begin;
         if (t_count == 1 || n < t_count) {
 #pragma omp master
@@ -243,13 +242,12 @@ template <typename T> class BatchParallelizer {
         };
 
         uint64_t local_count = 0;
-        for (int ot = 0; ot < t_count; ++ot) {
+        for (uint32_t ot = 0; ot < t_count; ++ot) {
             auto ot_counts = counts_of_thread(ot);
             auto j_begin = ot_counts[t];
             auto j_end = ot_counts[t + 1];
             for (size_t j = j_begin; j < j_end; ++j) {
                 auto i = m_batch_slots[j];
-                auto edge = *(begin + i);
                 func(*(begin + i));
             }
             local_count += j_end - j_begin;

include/dhb/block.h

@@ -5,27 +5,31 @@
 
 #include <algorithm>
 #include <cassert>
+#include <exception>
 #include <mutex>
-#include <sys/mman.h>
 #include <vector>
 
+#ifndef DHB_SYSTEM_ALLOCATOR
+#include <sys/mman.h>
+#endif
+
 namespace dhb {
 
 class BlockHandle;
 class BlockArray;
 
 using index_type = size_t;
-inline index_type illegalIndex() { return static_cast<index_type>(-1); };
-inline index_type tombstoneIndex() { return static_cast<index_type>(-2); };
+inline index_type illegalIndex() { return static_cast<index_type>(-1); }
+inline index_type tombstoneIndex() { return static_cast<index_type>(-2); }
 
 // TODO: This could take the entry size into account.
-inline bool uses_htab(size_t bsize) {
-    const size_t cache_line = 64;
-    // Each entry is typically 16 bytes or so.
-    // Preliminary experiments how that using the hash index is only useful if the block becomes
-    // larger than several cache lines (approx. 16 or so).
-    // Hence, the following is a reasonably good heuristic:
-    return 16 * bsize > 16 * cache_line;
+inline bool uses_htab(size_t const bsize) {
+    size_t constexpr cache_line = 64;
+    // Each entry is typically 16 bytes or so. Preliminary experiments show that
+    // using the hash index is only useful if the block becomes larger than
+    // several cache lines (approx. 16 or so). Hence, the following is a
+    // reasonably good heuristic:
+    return 16u * bsize > 16u * cache_line;
 }
 
 // Taken from https://stackoverflow.com/a/12996028.
@@ -416,7 +420,7 @@ template <typename E> class BlockState {
         if (uses_htab(m_bsize)) {
             index_type* htab = m_htab;
             auto h = hash_node(v);
-            index_type j;
+            index_type j = 0u;
             index_type ts = illegalIndex();
             for (index_type i = 0; true; ++i) {
                 if (i == m_bsize) {
@@ -437,8 +441,9 @@ template <typename E> class BlockState {
             }
 
             // If we did hit a tombstone, insert at the tombstone.
-            if (ts != illegalIndex())
+            if (ts != illegalIndex()) {
                 j = ts;
+            }
 
             // Insert into the adjacency list.
             auto i = m_degree++;
@@ -521,7 +526,7 @@ template <typename E> class BlockState {
         } else {
             // Find the index.
             index_type iv;
-            for (iv = 0; iv < m_degree; ++iv)
+            for (iv = 0u; iv < m_degree; ++iv)
                 if (m_entries[iv].vertex == v)
                     break;
 
@@ -530,7 +535,7 @@ template <typename E> class BlockState {
                 return false;
 
             // Swap with the last entry.
-            if (iv + 1 != m_degree)
+            if (iv + 1u != m_degree)
                 m_entries[iv] = m_entries[m_degree - 1];
 
             // Remove the last entry.

include/dhb/dynamic_hashed_blocks.h

@@ -9,6 +9,7 @@
 #include <functional>
 #include <memory>
 #include <mutex>
+#include <numeric>
 #include <omp.h>
 #include <vector>
 
@@ -55,7 +56,6 @@ template <typename E> struct Matrix {
             BlockState<E> new_block{new_bhandle, state};
             auto result = new_block.insert(v, ed);
 
-            auto old_block = std::move(m_graph->m_vertices[m_source]);
             auto old_bhandle = m_graph->m_handles[m_source];
             m_graph->m_vertices[m_source] = std::move(new_block);
             m_graph->m_handles[m_source] = new_bhandle;
@@ -174,7 +174,6 @@ template <typename E> struct Matrix {
             auto new_bhandle = m_manager->allocate_block(degree);
             BlockState<E> new_block{new_bhandle, associated_block};
 
-            auto old_block = std::move(m_vertices[u]);
             auto old_bhandle = m_handles[u];
             m_vertices[u] = std::move(new_block);
             m_handles[u] = new_bhandle;

include/dhb/vec.h

@@ -59,7 +59,6 @@ template <typename E> struct Vec {
         BlockState<E> new_block{new_bhandle, state};
         auto result = new_block.insert(u, ed);
 
-        auto old_block = std::move(m_state);
         auto old_bhandle = m_handle;
         m_state = std::move(new_block);
         m_handle = new_bhandle;

test/matrix_test.cpp

@@ -272,7 +272,6 @@ TEST_CASE("Matrix") {
         Edges new_edges{e89_14, e89_8, e89_13_update};
 
         auto cmp = [](Edge const& a, Edge const& b) { return a.source < b.source; };
-        auto key = [](Edge e) { return e.source; };
         auto fun = [&](Edge e) {
             std::tuple<dhb::BlockState<dhb::EdgeData>::iterator, bool> insertion_result =
                 m.neighbors(e.source).insert(e.target.vertex, e.target.data);
@@ -306,7 +305,6 @@ TEST_CASE("Matrix") {
         Matrix<EdgeData> m(graph::vertex_count(edges));
 
         auto cmp = [](Edge const& a, Edge const& b) { return a.source < b.source; };
-        auto key = [](Edge e) { return e.source; };
         auto fun = [&](Edge e) { m.neighbors(e.source).insert(e.target.vertex, e.target.data); };
         auto get_edge_f = [](Edge const& e) { return e.source; };
         BatchParallelizer<Edge> par;
@@ -318,7 +316,6 @@ TEST_CASE("Matrix") {
         Edges new_edges{e89_14, e89_8};
 
         auto cmp2 = [](Edge const& a, Edge const& b) { return a.source < b.source; };
-        auto key2 = [](Edge e) { return e.source; };
         par(new_edges.begin(), new_edges.end(), std::move(get_edge_f), std::move(cmp2),
             [&](Edge e) { m.neighbors(e.source).insert(e.target.vertex, e.target.data); });
 
@@ -351,7 +348,6 @@ TEST_CASE("Matrix") {
         Edges new_edges{e89_14, e89_8};
 
         auto cmp2 = [](Edge const& a, Edge const& b) { return a.source < b.source; };
-        auto key2 = [](Edge e) { return e.source; };
 
         par(new_edges.begin(), new_edges.end(), std::move(get_edge_f), std::move(cmp2),
             [&](Edge e) { m.neighbors(e.source).insert(e.target.vertex, e.target.data); });
@@ -372,9 +368,7 @@ TEST_CASE("Matrix") {
 
         Matrix<EdgeData> m(graph::vertex_count(edges));
 
-        auto cmp = [](Edge const& a, Edge const& b) { return a.source < b.source; };
         auto key = [](Edge e) { return e.source; };
-        auto fun = [&](Edge e) { m.neighbors(e.source).insert(e.target.vertex, e.target.data); };
         auto get_edge_f = [](Edge const& e) { return e.source; };
         BatchParallelizer<Edge> par;
 
@@ -401,7 +395,6 @@ TEST_CASE("Matrix") {
         Edges new_edges{e89_14, e89_8};
 
         auto cmp2 = [](Edge const& a, Edge const& b) { return a.source < b.source; };
-        auto key2 = [](Edge e) { return e.source; };
 
         par(new_edges.begin(), new_edges.end(), std::move(get_edge_f), std::move(cmp2),
             [&](Edge e) { m.neighbors(e.source).insert(e.target.vertex, e.target.data); });
@@ -423,7 +416,6 @@ TEST_CASE("Matrix") {
         Matrix<EdgeData> m(graph::vertex_count(edges));
 
         auto cmp = [](Edge const& a, Edge const& b) { return a.source < b.source; };
-        auto key = [](Edge e) { return e.source; };
         auto get_edge_f = [](Edge const& e) { return e.source; };
 
         BatchParallelizer<Edge> par;
@@ -437,9 +429,6 @@ TEST_CASE("Matrix") {
 
         Edges new_edges{e89_14, e89_8, e89_13_update};
 
-        auto cmp2 = [](Edge const& a, Edge const& b) { return a.source < b.source; };
-        auto key2 = [](Edge e) { return e.source; };
-
         par(new_edges.begin(), new_edges.end(), std::move(get_edge_f), std::move(cmp), [&](Edge e) {
             std::tuple<dhb::BlockState<dhb::EdgeData>::iterator, bool> insertion_result =
                 m.neighbors(e.source).insert(e.target.vertex, e.target.data);

test/submatrix.cpp → test/submatrix_test.cpp

@@ -15,9 +15,9 @@ class TestMatrix {
         // [ 13 | 4 | 0 ]
         // clang-format off
         dhb::Edges edges_a = { 
-            {0, Tar{0, ED{1.0}}},  {0, Tar{1, ED{3.0}}}, {0, Tar{2, ED{0.0}}},
-            {1, Tar{0, ED{4.0}}},  {1, Tar{1, ED{5.0}}}, {1, Tar{2, ED{8.0}}},
-            {2, Tar{0, ED{13.0}}}, {2, Tar{1, ED{4.0}}}, {2, Tar{2, ED{0.0}}},
+            {0, Tar{0, ED{1.0, 0}}},  {0, Tar{1, ED{3.0, 0}}}, {0, Tar{2, ED{0.0, 0}}},
+            {1, Tar{0, ED{4.0, 0}}},  {1, Tar{1, ED{5.0, 0}}}, {1, Tar{2, ED{8.0, 0}}},
+            {2, Tar{0, ED{13.0, 0}}}, {2, Tar{1, ED{4.0, 0}}}, {2, Tar{2, ED{0.0, 0}}},
         };
         // clang-format on