improbable-eng · improbable-til · Jan 18, 2022 · Jan 18, 2022 · Jan 18, 2022 · Jan 18, 2022
diff --git a/.bazelrc b/.bazelrc
@@ -29,6 +29,10 @@ build:ci --announce_rc
 build:linux --copt="-fvisibility=hidden"
 build:linux --copt="-fno-omit-frame-pointer"  # for friendlier stack traces
 build:linux --copt="-Wno-error"
+build:linux --copt="-Wall"
+build:linux --copt="-Wextra"
+build:linux --copt="-Werror=return-type"
+build:linux --copt="-Werror=switch"
 build:linux --copt="-mavx"
 build:linux --copt="-Wsequence-point"
 build:linux --copt="-Wsign-compare"

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,7 +5,15 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
-- Nothing yet.
+### Added
+- FilterSphere for filtering by sphere constraint (by ctbur)
+### Changed
+- Fixed imports  `<climits>` -> `<limits>` (by ctbur)
+- Fixed warnings:
+  - "unused" function argument warnings
+  - gcc/clang warnings
+  - MSVC warnings
+  - reserved identifier warnings (identifiers starting with `_`)
 
 ## [1.0.1] - 2021-05-06
 ### Changed

diff --git a/phtree/benchmark/count_mm_d_benchmark.cc b/phtree/benchmark/count_mm_d_benchmark.cc
@@ -79,7 +79,7 @@ class IndexBenchmark {
 
 template <dimension_t DIM, Scenario SCENARIO>
 IndexBenchmark<DIM, SCENARIO>::IndexBenchmark(benchmark::State& state, double avg_query_result_size)
-: data_type_{static_cast<const TestGenerator>(state.range(1))}
+: data_type_{static_cast<TestGenerator>(state.range(1))}
 , num_entities_(state.range(0))
 , avg_query_result_size_(avg_query_result_size)
 , tree_{}
@@ -118,7 +118,7 @@ void InsertEntry(
 }
 
 struct CounterTreeWithMap {
-    void operator()(const PhPointD<3>& key, const BucketType& value) {
+    void operator()(const PhPointD<3>&, const BucketType& value) {
         for (auto& x : value) {
             n_ += x.size();
         }

diff --git a/phtree/benchmark/erase_benchmark.cc b/phtree/benchmark/erase_benchmark.cc
@@ -91,7 +91,7 @@ void IndexBenchmark<DIM>::SetupWorld(benchmark::State& state) {
 }
 
 template <dimension_t DIM>
-void IndexBenchmark<DIM>::Insert(benchmark::State& state, PhTree<DIM, int>& tree) {
+void IndexBenchmark<DIM>::Insert(benchmark::State&, PhTree<DIM, int>& tree) {
     for (int i = 0; i < num_entities_; ++i) {
         tree.emplace(points_[i], i);
     }

diff --git a/phtree/benchmark/erase_d_benchmark.cc b/phtree/benchmark/erase_d_benchmark.cc
@@ -91,7 +91,7 @@ void IndexBenchmark<DIM>::SetupWorld(benchmark::State& state) {
 }
 
 template <dimension_t DIM>
-void IndexBenchmark<DIM>::Insert(benchmark::State& state, PhTreeD<DIM, int>& tree) {
+void IndexBenchmark<DIM>::Insert(benchmark::State&, PhTreeD<DIM, int>& tree) {
     for (int i = 0; i < num_entities_; ++i) {
         tree.emplace(points_[i], i);
     }

diff --git a/phtree/benchmark/extent_benchmark_weird.cc b/phtree/benchmark/extent_benchmark_weird.cc
@@ -104,7 +104,7 @@ class FilterBoxIntersection {
         max_include_bits = PRE(maxExclude);
     }
 
-    [[nodiscard]] bool IsEntryValid(const PhPoint<DIM>& key, const int& value) const {
+    [[nodiscard]] bool IsEntryValid(const PhPoint<DIM>& key, const int&) const {
         for (int i = 0; i < DIM; ++i) {
             if (key[i] < min_include_bits[i] || key[i] > max_include_bits[i]) {
                 return false;
@@ -146,11 +146,11 @@ class FilterTrue {
         maxIncludeBits = PRE(maxExclude);
     }
 
-    [[nodiscard]] bool IsEntryValid(const PhPoint<DIM>& key, const int& value) const {
+    [[nodiscard]] bool IsEntryValid(const PhPoint<DIM>&, const int&) const {
         return true;
     }
 
-    [[nodiscard]] bool IsNodeValid(const PhPoint<DIM>& prefix, int bits_to_ignore) const {
+    [[nodiscard]] bool IsNodeValid(const PhPoint<DIM>&, int) const {
         return true;
     }
 
@@ -164,11 +164,11 @@ class FilterTrue2 {
   public:
     FilterTrue2() : minIncludeBits{}, maxIncludeBits{} {};
 
-    [[nodiscard]] bool IsEntryValid(const PhPoint<DIM>& key, const int& value) const {
+    [[nodiscard]] bool IsEntryValid(const PhPoint<DIM>&, const int&) const {
         return true;
     }
 
-    [[nodiscard]] bool IsNodeValid(const PhPoint<DIM>& prefix, int bits_to_ignore) const {
+    [[nodiscard]] bool IsNodeValid(const PhPoint<DIM>&, int) const {
         return true;
     }
 
@@ -179,11 +179,11 @@ class FilterTrue2 {
 
 template <dimension_t DIM, typename T>
 struct FilterTrue3 {
-    [[nodiscard]] constexpr bool IsEntryValid(const PhPoint<DIM>& key, const T& value) const {
+    [[nodiscard]] constexpr bool IsEntryValid(const PhPoint<DIM>&, const T&) const {
         return true;
     }
 
-    [[nodiscard]] constexpr bool IsNodeValid(const PhPoint<DIM>& prefix, int bits_to_ignore) const {
+    [[nodiscard]] constexpr bool IsNodeValid(const PhPoint<DIM>&, int) const {
         return true;
     }
 };

diff --git a/phtree/benchmark/find_benchmark.cc b/phtree/benchmark/find_benchmark.cc
@@ -115,7 +115,7 @@ void IndexBenchmark<DIM>::SetupWorld(benchmark::State& state) {
 }
 
 template <dimension_t DIM>
-int IndexBenchmark<DIM>::QueryWorldCount(benchmark::State& state) {
+int IndexBenchmark<DIM>::QueryWorldCount(benchmark::State&) {
     static int pos = 0;
     pos = (pos + 1) % num_entities_;
     bool found = true;
@@ -130,7 +130,7 @@ int IndexBenchmark<DIM>::QueryWorldCount(benchmark::State& state) {
 }
 
 template <dimension_t DIM>
-int IndexBenchmark<DIM>::QueryWorldFind(benchmark::State& state) {
+int IndexBenchmark<DIM>::QueryWorldFind(benchmark::State&) {
     static int pos = 0;
     pos = (pos + 1) % num_entities_;
     bool found = true;

diff --git a/phtree/benchmark/query_box_d_benchmark.cc b/phtree/benchmark/query_box_d_benchmark.cc
@@ -120,7 +120,7 @@ void IndexBenchmark<DIM, QUERY_TYPE>::SetupWorld(benchmark::State& state) {
 
 template <dimension_t DIM, typename T>
 struct Counter {
-    void operator()(BoxType<DIM> key, T& t) {
+    void operator()(BoxType<DIM>, T&) {
         ++n_;
     }
 

diff --git a/phtree/benchmark/query_d_benchmark.cc b/phtree/benchmark/query_d_benchmark.cc
@@ -120,7 +120,7 @@ void IndexBenchmark<DIM, QUERY_TYPE>::SetupWorld(benchmark::State& state) {
 
 template <dimension_t DIM, typename T>
 struct Counter {
-    void operator()(PointType<DIM> key, T& t) {
+    void operator()(PointType<DIM>, T&) {
         ++n_;
     }
 

diff --git a/phtree/benchmark/query_mm_box_d_benchmark.cc b/phtree/benchmark/query_mm_box_d_benchmark.cc
@@ -80,7 +80,7 @@ class IndexBenchmark {
 
 template <dimension_t DIM, Scenario SCENARIO>
 IndexBenchmark<DIM, SCENARIO>::IndexBenchmark(benchmark::State& state, double avg_query_result_size)
-: data_type_{static_cast<const TestGenerator>(state.range(1))}
+: data_type_{static_cast<TestGenerator>(state.range(1))}
 , num_entities_(state.range(0))
 , avg_query_result_size_(avg_query_result_size)
 , tree_{}
@@ -126,7 +126,7 @@ bool CheckPosition(const payload_t& entity, const QueryBox& query) {
 }
 
 struct CounterTreeWithMap {
-    void operator()(const PhBoxD<3>& key, const BucketType& value) {
+    void operator()(const PhBoxD<3>&, const BucketType& value) {
         for (auto& x : value) {
             n_ += CheckPosition(x, box_);
         }
@@ -136,7 +136,7 @@ struct CounterTreeWithMap {
 };
 
 struct CounterMultiMap {
-    void operator()(const PhBoxD<3>& key, const payload_t& value) {
+    void operator()(const PhBoxD<3>&, const payload_t& value) {
         n_ += CheckPosition(value, box_);
     }
     const QueryBox& box_;

diff --git a/phtree/benchmark/query_mm_d_benchmark.cc b/phtree/benchmark/query_mm_d_benchmark.cc
@@ -82,7 +82,7 @@ class IndexBenchmark {
 
 template <dimension_t DIM, Scenario SCENARIO>
 IndexBenchmark<DIM, SCENARIO>::IndexBenchmark(benchmark::State& state, double avg_query_result_size)
-: data_type_{static_cast<const TestGenerator>(state.range(1))}
+: data_type_{static_cast<TestGenerator>(state.range(1))}
 , num_entities_(state.range(0))
 , avg_query_result_size_(avg_query_result_size)
 , tree_{}
@@ -129,7 +129,7 @@ bool CheckPosition(const payload_t& entity, const TestPoint& center, double radi
 }
 
 struct CounterTreeWithMap {
-    void operator()(const PhPointD<3>& key, const BucketType& value) {
+    void operator()(const PhPointD<3>&, const BucketType& value) {
         for (auto& x : value) {
             // n_ += (x.entity_id_ >= 0);
             n_ += CheckPosition(x, center_, radius_);
@@ -141,7 +141,7 @@ struct CounterTreeWithMap {
 };
 
 struct CounterMultiMap {
-    void operator()(const PhPointD<3>& key, const payload_t& value) {
+    void operator()(const PhPointD<3>&, const payload_t& value) {
         n_ += CheckPosition(value, center_, radius_);
     }
     const TestPoint& center_;

diff --git a/phtree/benchmark/update_mm_box_d_benchmark.cc b/phtree/benchmark/update_mm_box_d_benchmark.cc
@@ -87,7 +87,7 @@ class IndexBenchmark {
 template <dimension_t DIM, Scenario SCENARIO>
 IndexBenchmark<DIM, SCENARIO>::IndexBenchmark(
     benchmark::State& state, size_t updates_per_round, std::vector<double> move_distance)
-: data_type_{static_cast<const TestGenerator>(state.range(1))}
+: data_type_{static_cast<TestGenerator>(state.range(1))}
 , num_entities_(state.range(0))
 , updates_per_round_(updates_per_round)
 , move_distance_(std::move(move_distance))
@@ -143,7 +143,7 @@ typename std::enable_if<SCENARIO == Scenario::TREE_WITH_MAP, size_t>::type Updat
         assert(iter_old_bucket != tree.end());
         bool success = iter_old_bucket->erase(update.id_);
         if (iter_old_bucket->empty()) {
-            success &= tree.erase(iter_old_bucket);
+            success &= tree.erase(iter_old_bucket) != 0;
         }
         n += success;
     }
@@ -203,6 +203,7 @@ void IndexBenchmark<DIM, SCENARIO>::UpdateWorld(benchmark::State& state) {
     }
 
     if constexpr (SCENARIO == MULTI_MAP) {
+        (void)initial_tree_size;
         if (tree_.size() != num_entities_) {
             logging::error("Invalid index size after update: {}/{}", tree_.size(), num_entities_);
         }

diff --git a/phtree/benchmark/update_mm_d_benchmark.cc b/phtree/benchmark/update_mm_d_benchmark.cc
@@ -89,7 +89,7 @@ class IndexBenchmark {
 template <dimension_t DIM, Scenario SCENARIO>
 IndexBenchmark<DIM, SCENARIO>::IndexBenchmark(
     benchmark::State& state, size_t updates_per_round, std::vector<double> move_distance)
-: data_type_{static_cast<const TestGenerator>(state.range(1))}
+: data_type_{static_cast<TestGenerator>(state.range(1))}
 , num_entities_(state.range(0))
 , updates_per_round_(updates_per_round)
 , move_distance_(std::move(move_distance))
@@ -145,7 +145,7 @@ typename std::enable_if<SCENARIO == Scenario::TREE_WITH_MAP, size_t>::type Updat
         assert(iter_old_bucket != tree.end());
         bool success = iter_old_bucket->erase(update.id_);
         if (iter_old_bucket->empty()) {
-            success &= tree.erase(iter_old_bucket);
+            success &= tree.erase(iter_old_bucket) != 0;
         }
         n += success;
     }

diff --git a/phtree/common/filter.h b/phtree/common/filter.h
@@ -64,7 +64,7 @@ struct FilterNoOp {
      * @returns This default implementation always returns `true`.
      */
     template <typename KEY, typename T>
-    constexpr bool IsEntryValid(const KEY& key, const T& value) const {
+    constexpr bool IsEntryValid(const KEY& /*key*/, const T& /*value*/) const {
         return true;
     }
 
@@ -77,7 +77,7 @@ struct FilterNoOp {
      * @returns This default implementation always returns `true`.
      */
     template <typename KEY>
-    constexpr bool IsNodeValid(const KEY& prefix, int bits_to_ignore) const {
+    constexpr bool IsNodeValid(const KEY& /*prefix*/, int /*bits_to_ignore*/) const {
         return true;
     }
 };
@@ -116,7 +116,7 @@ class FilterAABB {
     }
 
     template <typename T>
-    [[nodiscard]] bool IsEntryValid(const KeyInternal& key, const T& value) const {
+    [[nodiscard]] bool IsEntryValid(const KeyInternal& key, const T& /*value*/) const {
         auto point = converter_.post(key);
         for (dimension_t i = 0; i < DIM; ++i) {
             if (point[i] < min_external_[i] || point[i] > max_external_[i]) {
@@ -151,13 +151,10 @@ class FilterAABB {
     const CONVERTER converter_;
 };
 
-
 /*
  * The sphere filter can be used to query a point tree for a sphere.
  */
-template <
-    typename CONVERTER = ConverterIEEE<3>,
-    typename DISTANCE = DistanceEuclidean<3>>
+template <typename CONVERTER = ConverterIEEE<3>, typename DISTANCE = DistanceEuclidean<3>>
 class FilterSphere {
     using KeyExternal = typename CONVERTER::KeyExternal;
     using KeyInternal = typename CONVERTER::KeyInternal;
@@ -177,7 +174,7 @@ class FilterSphere {
     , distance_function_{distance_function} {};
 
     template <typename T>
-    [[nodiscard]] bool IsEntryValid(const KeyInternal& key, const T& value) const {
+    [[nodiscard]] bool IsEntryValid(const KeyInternal& key, const T&) const {
         KeyExternal point = converter_.post(key);
         return distance_function_(center_external_, point) <= radius_;
     }

diff --git a/phtree/common/flat_array_map.h b/phtree/common/flat_array_map.h
@@ -95,14 +95,14 @@ class array_map {
         return PhFlatMapIterator<T, SIZE>{SIZE, *this};
     }
 
-    template <typename... _Args>
-    auto emplace(_Args&&... __args) {
-        return try_emplace_base(std::forward<_Args>(__args)...);
+    template <typename... Args>
+    auto emplace(Args&&... args) {
+        return try_emplace_base(std::forward<Args>(args)...);
     }
 
-    template <typename... _Args>
-    auto try_emplace(size_t index, _Args&&... __args) {
-        return try_emplace_base(index, std::forward<_Args>(__args)...);
+    template <typename... Args>
+    auto try_emplace(size_t index, Args&&... args) {
+        return try_emplace_base(index, std::forward<Args>(args)...);
     }
 
     bool erase(size_t index) {
@@ -123,13 +123,13 @@ class array_map {
     }
 
   private:
-    template <typename... _Args>
-    std::pair<PhFlatMapPair<T>*, bool> try_emplace_base(size_t index, _Args&&... __args) {
+    template <typename... Args>
+    std::pair<PhFlatMapPair<T>*, bool> try_emplace_base(size_t index, Args&&... args) {
         if (!occupied(index)) {
             new (reinterpret_cast<void*>(&data_[index])) PhFlatMapPair<T>(
                 std::piecewise_construct,
                 std::forward_as_tuple(index),
-                std::forward_as_tuple(std::forward<_Args>(__args)...));
+                std::forward_as_tuple(std::forward<Args>(args)...));
             occupied(index, true);
             return {&data(index), true};
         }
@@ -157,6 +157,7 @@ class array_map {
     }
 
     void occupied(size_t index, bool flag) {
+        (void)flag;
         assert(index < SIZE);
         assert(occupied(index) != flag);
         // flip the bit
@@ -183,7 +184,6 @@ class PhFlatMapIterator {
 
     explicit PhFlatMapIterator(size_t index, const array_map<T, SIZE>& map)
     : first{index}, map_{&map} {
-        assert(index >= 0);
         assert(index <= SIZE);
     }
 
@@ -192,7 +192,7 @@ class PhFlatMapIterator {
         return const_cast<PhFlatMapPair<T>&>(map_->data(first));
     }
 
-    auto* operator-> () const {
+    auto* operator->() const {
         assert(first < SIZE && map_->occupied(first));
         return const_cast<PhFlatMapPair<T>*>(&map_->data(first));
     }