issue 56 - support move-only/copy-only values

CHANGELOG.md

@@ -6,6 +6,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [Unreleased]
 ### Added
+- Added tested support for move-only and copy-only value objects.
+  [#56](https://github.com/tzaeschke/phtree-cpp/issues/56)
 - Added custom bucket implementation (similar to std::unordered_set). This improves update performance by 5%-20%.
   [#44](https://github.com/tzaeschke/phtree-cpp/issues/44)
 - Added `PhTree.relocate(old_key, new_key)` and `PhTree.relocate_if(old_key, new_key, predicate)`. 

phtree/BUILD

@@ -134,6 +134,32 @@ cc_test(
     ],
 )
 
+cc_test(
+    name = "phtree_d_test_copy_move",
+    timeout = "long",
+    srcs = [
+        "phtree_d_test_copy_move.cc",
+    ],
+    linkstatic = True,
+    deps = [
+        ":phtree",
+        "//phtree/testing/gtest_main",
+    ],
+)
+
+cc_test(
+    name = "phtree_multimap_d_test_copy_move",
+    timeout = "long",
+    srcs = [
+        "phtree_multimap_d_test_copy_move.cc",
+    ],
+    linkstatic = True,
+    deps = [
+        ":phtree",
+        "//phtree/testing/gtest_main",
+    ],
+)
+
 cc_test(
     name = "phtree_d_test_custom_key",
     timeout = "long",

phtree/common/flat_sparse_map.h

@@ -32,7 +32,7 @@ namespace improbable::phtree {
 
 namespace {
 template <typename T>
-using PhFlatMapPair = std::pair<size_t, T>;
+using PhSparseMapPair = std::pair<size_t, T>;
 
 using index_t = std::int32_t;
 }  // namespace
@@ -68,14 +68,14 @@ class sparse_map {
 
     [[nodiscard]] auto lower_bound(size_t key) {
         return std::lower_bound(
-            data_.begin(), data_.end(), key, [](PhFlatMapPair<T>& left, const size_t key) {
+            data_.begin(), data_.end(), key, [](PhSparseMapPair<T>& left, const size_t key) {
                 return left.first < key;
             });
     }
 
     [[nodiscard]] auto lower_bound(size_t key) const {
         return std::lower_bound(
-            data_.cbegin(), data_.cend(), key, [](const PhFlatMapPair<T>& left, const size_t key) {
+            data_.cbegin(), data_.cend(), key, [](const PhSparseMapPair<T>& left, const size_t key) {
                 return left.first < key;
             });
     }
@@ -117,7 +117,7 @@ class sparse_map {
         }
     }
 
-    void erase(const typename std::vector<PhFlatMapPair<T>>::iterator& iterator) {
+    void erase(const typename std::vector<PhSparseMapPair<T>>::iterator& iterator) {
         data_.erase(iterator);
     }
 
@@ -151,7 +151,7 @@ class sparse_map {
         }
     }
 
-    std::vector<PhFlatMapPair<T>> data_;
+    std::vector<PhSparseMapPair<T>> data_;
 };
 
 }  // namespace improbable::phtree

phtree/phtree_d_test_copy_move.cc

@@ -0,0 +1,298 @@
+/*
+ * Copyright 2020 Improbable Worlds Limited
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "phtree/phtree.h"
+#include <gtest/gtest.h>
+#include <random>
+
+using namespace improbable::phtree;
+
+template <dimension_t DIM>
+using TestPoint = PhPointD<DIM>;
+
+template <dimension_t DIM, typename T>
+using TestTree = PhTreeD<DIM, T>;
+
+class DoubleRng {
+  public:
+    DoubleRng(double minIncl, double maxExcl) : eng(), rnd{minIncl, maxExcl} {}
+
+    double next() {
+        return rnd(eng);
+    }
+
+  private:
+    std::default_random_engine eng;
+    std::uniform_real_distribution<double> rnd;
+};
+
+struct IdCopyOnly {
+    explicit IdCopyOnly(const size_t i) : _i{i} {}
+
+    IdCopyOnly() = default;
+    IdCopyOnly(const IdCopyOnly& other) = default;
+    IdCopyOnly(IdCopyOnly&& other) = delete;
+    // IdCopyOnly& operator=(const IdCopyOnly& other) = default;
+    IdCopyOnly& operator=(const IdCopyOnly& other) {
+        _i = other._i;
+        return *this;
+    }
+    IdCopyOnly& operator=(IdCopyOnly&& other) = delete;
+    ~IdCopyOnly() = default;
+
+    bool operator==(const IdCopyOnly& rhs) const {
+        return _i == rhs._i;
+    }
+
+    size_t _i{};
+};
+
+struct IdMoveOnly {
+    explicit IdMoveOnly(const size_t i) : _i{i} {}
+
+    IdMoveOnly() = default;
+    IdMoveOnly(const IdMoveOnly& other) = delete;
+    IdMoveOnly(IdMoveOnly&& other) = default;
+    IdMoveOnly& operator=(const IdMoveOnly& other) = delete;
+    IdMoveOnly& operator=(IdMoveOnly&& other) = default;
+    ~IdMoveOnly() = default;
+
+    bool operator==(const IdMoveOnly& rhs) const {
+        return _i == rhs._i;
+    }
+
+    size_t _i{};
+};
+
+// Assert that copy-ctr is not called even when available
+struct IdCopyOrMove {
+    explicit IdCopyOrMove(const size_t i) : _i{i} {}
+
+    IdCopyOrMove() = default;
+    IdCopyOrMove(const IdCopyOrMove&) {
+        assert(false);
+    }
+    IdCopyOrMove(IdCopyOrMove&& other) = default;
+    IdCopyOrMove& operator=(const IdCopyOrMove&) {
+        assert(false);
+    }
+    IdCopyOrMove& operator=(IdCopyOrMove&& other) = default;
+    ~IdCopyOrMove() = default;
+
+    bool operator==(const IdCopyOrMove& rhs) const {
+        return _i == rhs._i;
+    }
+
+    size_t _i{};
+};
+
+template <dimension_t DIM>
+void generateCube(std::vector<TestPoint<DIM>>& points, size_t N) {
+    DoubleRng rng(-1000, 1000);
+    auto refTree = std::map<TestPoint<DIM>, size_t>();
+
+    points.reserve(N);
+    for (size_t i = 0; i < N; i++) {
+        TestPoint<DIM> point{};
+        for (dimension_t d = 0; d < DIM; ++d) {
+            point[d] = rng.next();
+        }
+        if (refTree.count(point) != 0) {
+            i--;
+            continue;
+        }
+
+        refTree.emplace(point, i);
+        points.push_back(point);
+    }
+    ASSERT_EQ(refTree.size(), N);
+    ASSERT_EQ(points.size(), N);
+}
+
+template <dimension_t DIM, typename Id>
+void SmokeTestBasicOps_QueryAndErase(TestTree<DIM, Id>& tree, std::vector<TestPoint<DIM>>& points) {
+    size_t N = points.size();
+
+    for (size_t i = 0; i < N; i++) {
+        TestPoint<DIM>& p = points.at(i);
+        auto q = tree.begin_query({p, p});
+        ASSERT_NE(q, tree.end());
+        ASSERT_EQ(i, (*q)._i);
+        q++;
+        ASSERT_EQ(q, tree.end());
+    }
+
+    for (size_t i = 0; i < N; i++) {
+        TestPoint<DIM>& p = points.at(i);
+        auto q = tree.begin_knn_query(1, p, DistanceEuclidean<DIM>());
+        ASSERT_NE(q, tree.end());
+        ASSERT_EQ(i, (*q)._i);
+        q++;
+        ASSERT_EQ(q, tree.end());
+    }
+
+    // TODO enable for new relocate functions
+    //    for (size_t i = 0; i < N; i++) {
+    //        TestPoint<DIM>& p = points.at(i);
+    //        TestPoint<DIM> pOld = p;
+    //        for (dimension_t d = 0; d < DIM; ++d) {
+    //            p[d] += 10000;
+    //        }
+    //        auto r = tree.relocate(pOld, p);
+    //        ASSERT_EQ(r, 1u);
+    //    }
+
+    PhTreeDebugHelper::CheckConsistency(tree);
+
+    for (size_t i = 0; i < N; i++) {
+        TestPoint<DIM>& p = points.at(i);
+        ASSERT_NE(tree.find(p), tree.end());
+        ASSERT_EQ(tree.count(p), 1u);
+        ASSERT_EQ(i, tree.find(p)->_i);
+        if (i % 2 == 0) {
+            ASSERT_EQ(1u, tree.erase(p));
+        } else {
+            auto iter = tree.find(p);
+            ASSERT_EQ(1u, tree.erase(iter));
+        }
+
+        ASSERT_EQ(tree.count(p), 0u);
+        ASSERT_EQ(tree.end(), tree.find(p));
+        ASSERT_EQ(N - i - 1, tree.size());
+
+        // try remove again
+        ASSERT_EQ(0u, tree.erase(p));
+        ASSERT_EQ(tree.count(p), 0u);
+        ASSERT_EQ(tree.end(), tree.find(p));
+        ASSERT_EQ(N - i - 1, tree.size());
+        if (i < N - 1) {
+            ASSERT_FALSE(tree.empty());
+        }
+    }
+    ASSERT_EQ(0u, tree.size());
+    ASSERT_TRUE(tree.empty());
+    PhTreeDebugHelper::CheckConsistency(tree);
+}
+
+template <dimension_t DIM, typename Id>
+void SmokeTestBasicOps(size_t N) {
+    TestTree<DIM, Id> tree;
+    std::vector<TestPoint<DIM>> points;
+    generateCube(points, N);
+
+    ASSERT_EQ(0u, tree.size());
+    ASSERT_TRUE(tree.empty());
+    PhTreeDebugHelper::CheckConsistency(tree);
+
+    for (size_t i = 0; i < N; i++) {
+        TestPoint<DIM>& p = points.at(i);
+        ASSERT_EQ(tree.count(p), 0u);
+        ASSERT_EQ(tree.end(), tree.find(p));
+
+        Id id(i);
+        if (i % 4 == 0) {
+            ASSERT_TRUE(tree.try_emplace(p, id).second);
+        } else if (i % 4 == 1) {
+            ASSERT_TRUE(tree.emplace(p, id).second);
+        } else if (i % 4 == 2) {
+            tree[p] = id;
+        } else {
+            ASSERT_TRUE(tree.insert(p, id).second);
+        }
+        ASSERT_EQ(tree.count(p), 1u);
+        ASSERT_NE(tree.end(), tree.find(p));
+        ASSERT_EQ(id._i, tree.find(p)->_i);
+        ASSERT_EQ(i + 1, tree.size());
+
+        // try adding it again
+        ASSERT_FALSE(tree.insert(p, id).second);
+        ASSERT_FALSE(tree.emplace(p, id).second);
+        ASSERT_EQ(tree.count(p), 1u);
+        ASSERT_NE(tree.end(), tree.find(p));
+        ASSERT_EQ(id._i, tree.find(p)->_i);
+        ASSERT_EQ(i + 1, tree.size());
+        ASSERT_FALSE(tree.empty());
+    }
+
+    SmokeTestBasicOps_QueryAndErase(tree, points);
+}
+
+TEST(PhTreeDTestCopyMove, SmokeTestBasicOpsCopyOnly) {
+    SmokeTestBasicOps<1, IdCopyOnly>(100);
+    SmokeTestBasicOps<3, IdCopyOnly>(100);
+    SmokeTestBasicOps<6, IdCopyOnly>(100);
+    SmokeTestBasicOps<10, IdCopyOnly>(100);
+    SmokeTestBasicOps<20, IdCopyOnly>(100);
+    SmokeTestBasicOps<63, IdCopyOnly>(100);
+}
+
+template <dimension_t DIM, typename Id>
+void SmokeTestBasicOpsMoveOnly(size_t N) {
+    TestTree<DIM, Id> tree;
+    std::vector<TestPoint<DIM>> points;
+    generateCube(points, N);
+
+    ASSERT_EQ(0u, tree.size());
+    ASSERT_TRUE(tree.empty());
+    PhTreeDebugHelper::CheckConsistency(tree);
+
+    for (size_t i = 0; i < N; i++) {
+        TestPoint<DIM>& p = points.at(i);
+        ASSERT_EQ(tree.count(p), 0u);
+        ASSERT_EQ(tree.end(), tree.find(p));
+
+        if (i % 2 == 0) {
+            ASSERT_TRUE(tree.try_emplace(p, Id(i)).second);
+        } else if (i % 4 == 1) {
+            tree[p] = Id(i);
+        } else {
+            ASSERT_TRUE(tree.emplace(p, Id(i)).second);
+        }
+        ASSERT_EQ(tree.count(p), 1u);
+        ASSERT_NE(tree.end(), tree.find(p));
+        ASSERT_EQ(i, tree.find(p)->_i);
+        ASSERT_EQ(i + 1, tree.size());
+
+        // try adding it again
+        ASSERT_FALSE(tree.try_emplace(p, Id(i)).second);
+        ASSERT_FALSE(tree.emplace(p, Id(i)).second);
+        ASSERT_EQ(tree.count(p), 1u);
+        ASSERT_NE(tree.end(), tree.find(p));
+        ASSERT_EQ(i, tree.find(p)->_i);
+        ASSERT_EQ(i + 1, tree.size());
+        ASSERT_FALSE(tree.empty());
+    }
+
+    SmokeTestBasicOps_QueryAndErase(tree, points);
+}
+
+TEST(PhTreeDTestCopyMove, SmokeTestBasicOpsMoveOnly) {
+    SmokeTestBasicOpsMoveOnly<1, IdMoveOnly>(100);
+    SmokeTestBasicOpsMoveOnly<3, IdMoveOnly>(100);
+    SmokeTestBasicOpsMoveOnly<6, IdMoveOnly>(100);
+    SmokeTestBasicOpsMoveOnly<10, IdMoveOnly>(100);
+    SmokeTestBasicOpsMoveOnly<20, IdMoveOnly>(100);
+    SmokeTestBasicOpsMoveOnly<63, IdMoveOnly>(100);
+}
+
+TEST(PhTreeDTestCopyMove, SmokeTestBasicOpsCopyFails) {
+    SmokeTestBasicOpsMoveOnly<1, IdCopyOrMove>(100);
+    SmokeTestBasicOpsMoveOnly<3, IdCopyOrMove>(100);
+    SmokeTestBasicOpsMoveOnly<6, IdCopyOrMove>(100);
+    SmokeTestBasicOpsMoveOnly<10, IdCopyOrMove>(100);
+    SmokeTestBasicOpsMoveOnly<20, IdCopyOrMove>(100);
+    SmokeTestBasicOpsMoveOnly<63, IdCopyOrMove>(100);
+}