Additional Compiler Warnings (#73)

Author: zfergus

cmake/ipc_toolkit/ipc_toolkit_warnings.cmake

@@ -10,20 +10,22 @@ endif()
 set(IPC_TOOLKIT_WARNING_FLAGS
   -Wall
   -Wextra
-  -pedantic
+  -Wpedantic
+  # -Werror
 
   # -Wconversion
+  # -Wsign-conversion
   # -Wunsafe-loop-optimizations # broken with C++11 loops
   -Wunused
 
-  -Wno-long-long
+  -Wno-long-long # disable warnings about using long long
   -Wpointer-arith
   -Wformat=2
   -Wuninitialized
   -Wcast-qual
-  # -Wmissing-noreturn
+  -Wmissing-noreturn
   -Wmissing-format-attribute
-  # -Wredundant-decls
+  -Wredundant-decls
 
   -Werror=implicit
   -Werror=nonnull
@@ -43,11 +45,10 @@ set(IPC_TOOLKIT_WARNING_FLAGS
   -Wunused-but-set-variable
   -Wno-unused-parameter
 
-  #-Weffc++
+  # -Weffc++
   -Wold-style-cast
-  # -Wsign-conversion
 
-  # -Wshadow
+  -Wshadow
 
   -Wstrict-null-sentinel
   -Woverloaded-virtual
@@ -60,7 +61,7 @@ set(IPC_TOOLKIT_WARNING_FLAGS
   # lacks a case for one or more of the named codes of that enumeration.
   -Wswitch
   # This is annoying if all cases are already covered.
-  # -Wswitch-default
+  -Wswitch-default
   # This is annoying if there is a default that covers the rest.
   # -Wswitch-enum
   -Wswitch-unreachable
@@ -70,20 +71,17 @@ set(IPC_TOOLKIT_WARNING_FLAGS
   -Wdisabled-optimization
   # -Winline # produces warning on default implicit destructor
   -Winvalid-pch
-  # -Wmissing-include-dirs
+  -Wmissing-include-dirs
   -Wpacked
   -Wno-padded
   -Wstrict-overflow
   -Wstrict-overflow=2
 
-  # -Wctor-dtor-privacy
+  -Wctor-dtor-privacy
   -Wlogical-op
-  # -Wnoexcept
   -Woverloaded-virtual
   # -Wundef
 
-  -Wnon-virtual-dtor
-  -Wdelete-non-virtual-dtor
   -Werror=non-virtual-dtor
   -Werror=delete-non-virtual-dtor
 
@@ -141,8 +139,6 @@ set(IPC_TOOLKIT_WARNING_FLAGS
   -fno-omit-frame-pointer
   -fno-optimize-sibling-calls
 
-  -Wno-pedantic
-
   -Wno-redundant-decls
 )
 

src/ipc/broad_phase/aabb.cpp

@@ -7,7 +7,9 @@
 
 namespace ipc {
 
-AABB::AABB(const ArrayMax3d& min, const ArrayMax3d& max) : min(min), max(max)
+AABB::AABB(const ArrayMax3d& _min, const ArrayMax3d& _max)
+    : min(_min)
+    , max(_max)
 {
     assert(min.size() == max.size());
     assert((min <= max).all());

src/ipc/broad_phase/hash_grid.cpp

@@ -9,7 +9,7 @@
 #include <tbb/parallel_for.h>
 #include <tbb/parallel_sort.h>
 
-#include <algorithm> // std::min/max
+#include <algorithm>                          // std::min/max
 
 #define IPC_TOOLKIT_HASH_GRID_USE_SORT_UNIQUE // else use unordered_set
 
@@ -149,20 +149,22 @@ void HashGrid::detect_candidates(
     size_t num_items = items0.size() + items1.size();
     std::vector<long> merged_item_indices;
     merged_item_indices.reserve(num_items);
-    long i = 0, j = 0;
-    while (i < items0.size() && j < items1.size()) {
-        if (items0[i] < items1[j]) {
+    {
+        long i = 0, j = 0;
+        while (i < items0.size() && j < items1.size()) {
+            if (items0[i] < items1[j]) {
+                merged_item_indices.push_back(-(i++) - 1);
+            } else {
+                merged_item_indices.push_back(j++);
+            }
+        }
+        while (i < items0.size()) {
             merged_item_indices.push_back(-(i++) - 1);
-        } else {
+        }
+        while (j < items1.size()) {
             merged_item_indices.push_back(j++);
         }
     }
-    while (i < items0.size()) {
-        merged_item_indices.push_back(-(i++) - 1);
-    }
-    while (j < items1.size()) {
-        merged_item_indices.push_back(j++);
-    }
     assert(merged_item_indices.size() == num_items);
 
     const auto get_item = [&](long i) -> const HashItem& {

src/ipc/broad_phase/hash_grid.hpp

@@ -10,7 +10,7 @@ struct HashItem {
     long id;  /// @brief The value of the item.
 
     /// @brief Construct a hash item as a (key, value) pair.
-    HashItem(int key, int id) : key(key), id(id) { }
+    HashItem(int _key, int _id) : key(_key), id(_id) { }
 
     /// @brief Compare HashItems by their keys for sorting.
     bool operator<(const HashItem& other) const

src/ipc/broad_phase/sweep_and_tiniest_queue.cpp

@@ -12,21 +12,21 @@ namespace ipc {
 
 void SweepAndTiniestQueue::build(
     const Eigen::MatrixXd& vertices,
-    const Eigen::MatrixXi& edges,
-    const Eigen::MatrixXi& faces,
+    const Eigen::MatrixXi& _edges,
+    const Eigen::MatrixXi& _faces,
     double inflation_radius)
 {
-    build(vertices, vertices, edges, faces, inflation_radius);
+    build(vertices, vertices, _edges, _faces, inflation_radius);
 }
 
 void SweepAndTiniestQueue::build(
     const Eigen::MatrixXd& vertices_t0,
     const Eigen::MatrixXd& vertices_t1,
-    const Eigen::MatrixXi& edges,
-    const Eigen::MatrixXi& faces,
+    const Eigen::MatrixXi& _edges,
+    const Eigen::MatrixXi& _faces,
     double inflation_radius)
 {
-    CopyMeshBroadPhase::copy_mesh(edges, faces);
+    CopyMeshBroadPhase::copy_mesh(_edges, _faces);
     num_vertices = vertices_t0.rows();
     stq::cpu::constructBoxes(
         vertices_t0, vertices_t1, edges, faces, boxes, inflation_radius);
@@ -122,23 +122,23 @@ bool SweepAndTiniestQueue::is_face(long id) const
 #ifdef IPC_TOOLKIT_WITH_CUDA
 void SweepAndTiniestQueueGPU::build(
     const Eigen::MatrixXd& vertices,
-    const Eigen::MatrixXi& edges,
-    const Eigen::MatrixXi& faces,
+    const Eigen::MatrixXi& _edges,
+    const Eigen::MatrixXi& _faces,
     double inflation_radius)
 {
-    CopyMeshBroadPhase::copy_mesh(edges, faces);
+    CopyMeshBroadPhase::copy_mesh(_edges, _faces);
     ccd::gpu::construct_static_collision_candidates(
         vertices, edges, faces, overlaps, boxes, inflation_radius);
 }
 
 void SweepAndTiniestQueueGPU::build(
     const Eigen::MatrixXd& vertices_t0,
     const Eigen::MatrixXd& vertices_t1,
-    const Eigen::MatrixXi& edges,
-    const Eigen::MatrixXi& faces,
+    const Eigen::MatrixXi& _edges,
+    const Eigen::MatrixXi& _faces,
     double inflation_radius)
 {
-    CopyMeshBroadPhase::copy_mesh(edges, faces);
+    CopyMeshBroadPhase::copy_mesh(_edges, _faces);
     ccd::gpu::construct_continuous_collision_candidates(
         vertices_t0, vertices_t1, edges, faces, overlaps, boxes,
         inflation_radius);

src/ipc/broad_phase/sweep_and_tiniest_queue.hpp

@@ -59,12 +59,12 @@ class SweepAndTiniestQueue : public CopyMeshBroadPhase {
 
     /// @brief Find the candidate vertex-vertex collisions.
     /// @param[out] candidates The candidate vertex-vertex collisisons.
-    void detect_vertex_vertex_candidates(
+    [[noreturn]] void detect_vertex_vertex_candidates(
         std::vector<VertexVertexCandidate>& candidates) const override;
 
     /// @brief Find the candidate edge-vertex collisisons.
     /// @param[out] candidates The candidate edge-vertex collisisons.
-    void detect_edge_vertex_candidates(
+    [[noreturn]] void detect_edge_vertex_candidates(
         std::vector<EdgeVertexCandidate>& candidates) const override;
 
     /// @brief Find the candidate edge-edge collisions.
@@ -79,7 +79,7 @@ class SweepAndTiniestQueue : public CopyMeshBroadPhase {
 
     /// @brief Find the candidate edge-face intersections.
     /// @param[out] candidates The candidate edge-face intersections.
-    void detect_edge_face_candidates(
+    [[noreturn]] void detect_edge_face_candidates(
         std::vector<EdgeFaceCandidate>& candidates) const override;
 
 protected:
@@ -127,12 +127,12 @@ class SweepAndTiniestQueueGPU : public CopyMeshBroadPhase {
 
     /// @brief Find the candidate vertex-vertex collisions.
     /// @param[out] candidates The candidate vertex-vertex collisisons.
-    void detect_vertex_vertex_candidates(
+    [[noreturn]] void detect_vertex_vertex_candidates(
         std::vector<VertexVertexCandidate>& candidates) const override;
 
     /// @brief Find the candidate edge-vertex collisisons.
     /// @param[out] candidates The candidate edge-vertex collisisons.
-    void detect_edge_vertex_candidates(
+    [[noreturn]] void detect_edge_vertex_candidates(
         std::vector<EdgeVertexCandidate>& candidates) const override;
 
     /// @brief Find the candidate edge-edge collisions.
@@ -147,7 +147,7 @@ class SweepAndTiniestQueueGPU : public CopyMeshBroadPhase {
 
     /// @brief Find the candidate edge-face intersections.
     /// @param[out] candidates The candidate edge-face intersections.
-    void detect_edge_face_candidates(
+    [[noreturn]] void detect_edge_face_candidates(
         std::vector<EdgeFaceCandidate>& candidates) const override;
 
 private:

src/ipc/broad_phase/voxel_size_heuristic.cpp

@@ -86,15 +86,15 @@ double mean_edge_length(
 
     double sum = 0;
     for (int i = 0; i < edges.rows(); i++) {
-        const size_t e0i = edges(i, 0), e1i = edges(i, 1);
+        const int e0i = edges(i, 0), e1i = edges(i, 1);
         sum += (vertices_t0.row(e0i) - vertices_t0.row(e1i)).norm();
         sum += (vertices_t1.row(e0i) - vertices_t1.row(e1i)).norm();
     }
     const double mean = sum / (2 * edges.rows());
 
     std_deviation = 0;
     for (int i = 0; i < edges.rows(); i++) {
-        const size_t e0i = edges(i, 0), e1i = edges(i, 1);
+        const int e0i = edges(i, 0), e1i = edges(i, 1);
         std_deviation += std::pow(
             (vertices_t0.row(e0i) - vertices_t0.row(e1i)).norm() - mean, 2);
         std_deviation += std::pow(

src/ipc/candidates/candidates.cpp

@@ -13,11 +13,13 @@
 
 namespace ipc {
 
-bool implements_vertex_vertex(const BroadPhaseMethod method)
-{
-    return method != BroadPhaseMethod::SWEEP_AND_TINIEST_QUEUE
-        && method != BroadPhaseMethod::SWEEP_AND_TINIEST_QUEUE_GPU;
-}
+namespace {
+    bool implements_vertex_vertex(const BroadPhaseMethod method)
+    {
+        return method != BroadPhaseMethod::SWEEP_AND_TINIEST_QUEUE
+            && method != BroadPhaseMethod::SWEEP_AND_TINIEST_QUEUE_GPU;
+    }
+} // namespace
 
 void Candidates::build(
     const CollisionMesh& mesh,

src/ipc/candidates/edge_edge.cpp

@@ -5,9 +5,9 @@
 
 namespace ipc {
 
-EdgeEdgeCandidate::EdgeEdgeCandidate(long edge0_id, long edge1_id)
-    : edge0_id(edge0_id)
-    , edge1_id(edge1_id)
+EdgeEdgeCandidate::EdgeEdgeCandidate(long _edge0_id, long _edge1_id)
+    : edge0_id(_edge0_id)
+    , edge1_id(_edge1_id)
 {
 }
 

src/ipc/candidates/edge_face.cpp

@@ -2,9 +2,9 @@
 
 namespace ipc {
 
-EdgeFaceCandidate::EdgeFaceCandidate(long edge_id, long face_id)
-    : edge_id(edge_id)
-    , face_id(face_id)
+EdgeFaceCandidate::EdgeFaceCandidate(long _edge_id, long _face_id)
+    : edge_id(_edge_id)
+    , face_id(_face_id)
 {
 }