diff --git a/gtsam/base/timing.cpp b/gtsam/base/timing.cpp index 9e9d1d1134..ff68ad4d19 100644 --- a/gtsam/base/timing.cpp +++ b/gtsam/base/timing.cpp @@ -109,7 +109,7 @@ void TimingOutline::print(const std::string& outline) const { /* ************************************************************************* */ void TimingOutline::printCsvHeader(bool addLineBreak) const { -#ifdef GTSAM_USE_BOOST_FEATURES +#if GTSAM_USE_BOOST_FEATURES // Order is (CPU time, number of times, wall time, time + children in seconds, // min time, max time) std::cout << label_ + " cpu time (s)" << "," << label_ + " #calls" << "," @@ -134,7 +134,7 @@ void TimingOutline::printCsvHeader(bool addLineBreak) const { /* ************************************************************************* */ void TimingOutline::printCsv(bool addLineBreak) const { -#ifdef GTSAM_USE_BOOST_FEATURES +#if GTSAM_USE_BOOST_FEATURES // Order is (CPU time, number of times, wall time, time + children in seconds, // min time, max time) std::cout << self() << "," << n_ << "," << wall() << "," << secs() << "," diff --git a/gtsam/constrained/NonlinearConstraint.h b/gtsam/constrained/NonlinearConstraint.h index a01430bf36..f64e8bfc1d 100644 --- a/gtsam/constrained/NonlinearConstraint.h +++ b/gtsam/constrained/NonlinearConstraint.h @@ -21,7 +21,7 @@ #include #include #include -#ifdef GTSAM_ENABLE_BOOST_SERIALIZATION +#if GTSAM_ENABLE_BOOST_SERIALIZATION #include #endif @@ -77,7 +77,7 @@ class GTSAM_EXPORT NonlinearConstraint : public NoiseModelFactor { } private: -#ifdef GTSAM_ENABLE_BOOST_SERIALIZATION +#if GTSAM_ENABLE_BOOST_SERIALIZATION /** Serialization function */ friend class boost::serialization::access; template diff --git a/gtsam/constrained/NonlinearEqualityConstraint.h b/gtsam/constrained/NonlinearEqualityConstraint.h index 017ca16077..f8b0205a11 100644 --- a/gtsam/constrained/NonlinearEqualityConstraint.h +++ b/gtsam/constrained/NonlinearEqualityConstraint.h @@ -39,7 +39,7 @@ class GTSAM_EXPORT NonlinearEqualityConstraint : public NonlinearConstraint { virtual ~NonlinearEqualityConstraint() {} private: -#ifdef GTSAM_ENABLE_BOOST_SERIALIZATION +#if GTSAM_ENABLE_BOOST_SERIALIZATION /** Serialization function */ friend class boost::serialization::access; template @@ -85,7 +85,7 @@ class ExpressionEqualityConstraint : public NonlinearEqualityConstraint { } private: -#ifdef GTSAM_ENABLE_BOOST_SERIALIZATION +#if GTSAM_ENABLE_BOOST_SERIALIZATION /** Serialization function */ friend class boost::serialization::access; template @@ -131,7 +131,7 @@ class GTSAM_EXPORT ZeroCostConstraint : public NonlinearEqualityConstraint { } private: -#ifdef GTSAM_ENABLE_BOOST_SERIALIZATION +#if GTSAM_ENABLE_BOOST_SERIALIZATION /** Serialization function */ friend class boost::serialization::access; template @@ -166,7 +166,7 @@ class GTSAM_EXPORT NonlinearEqualityConstraints : public FactorGraph diff --git a/gtsam/constrained/NonlinearInequalityConstraint.h b/gtsam/constrained/NonlinearInequalityConstraint.h index 9dbf1f008d..edc2b90e99 100644 --- a/gtsam/constrained/NonlinearInequalityConstraint.h +++ b/gtsam/constrained/NonlinearInequalityConstraint.h @@ -61,7 +61,7 @@ class GTSAM_EXPORT NonlinearInequalityConstraint : public NonlinearConstraint { virtual NoiseModelFactor::shared_ptr penaltyFactorEquality(const double mu = 1.0) const; private: -#ifdef GTSAM_ENABLE_BOOST_SERIALIZATION +#if GTSAM_ENABLE_BOOST_SERIALIZATION /** Serialization function */ friend class boost::serialization::access; template @@ -129,7 +129,7 @@ class GTSAM_EXPORT ScalarExpressionInequalityConstraint : public NonlinearInequa } private: -#ifdef GTSAM_ENABLE_BOOST_SERIALIZATION +#if GTSAM_ENABLE_BOOST_SERIALIZATION /** Serialization function */ friend class boost::serialization::access; template @@ -169,7 +169,7 @@ class GTSAM_EXPORT NonlinearInequalityConstraints const double mu = 1.0) const; private: -#ifdef GTSAM_ENABLE_BOOST_SERIALIZATION +#if GTSAM_ENABLE_BOOST_SERIALIZATION /** Serialization function */ friend class boost::serialization::access; template diff --git a/gtsam/inference/BayesTree-inst.h b/gtsam/inference/BayesTree-inst.h index fdfa94d2b6..0648a90f64 100644 --- a/gtsam/inference/BayesTree-inst.h +++ b/gtsam/inference/BayesTree-inst.h @@ -196,21 +196,20 @@ namespace gtsam { for (auto&& root: roots_) { std::queue bfs_queue; - // first, move the root to the queue - bfs_queue.push(root); - root = nullptr; // now the root node is owned by the queue + // first, steal the root and move it to the queue. This invalidates root + bfs_queue.push(std::move(root)); // do a BFS on the tree, for each node, add its children to the queue, and then delete it from the queue // So if the reference count of the node is 1, it will be deleted, and because its children are in the queue, // the deletion of the node will not trigger a recursive deletion of the children. while (!bfs_queue.empty()) { - // move the ownership of the front node from the queue to the current variable - auto current = bfs_queue.front(); + // move the ownership of the front node from the queue to the current variable, invalidating the sharedClique at the front of the queue + auto current = std::move(bfs_queue.front()); bfs_queue.pop(); // add the children of the current node to the queue, so that the queue will also own the children nodes. for (auto child: current->children) { - bfs_queue.push(child); + bfs_queue.push(std::move(child)); } // leaving the scope of current will decrease the reference count of the current node by 1, and if the reference count is 0, // the node will be deleted. Because the children are in the queue, the deletion of the node will not trigger a recursive // deletion of the children. diff --git a/gtsam/inference/ClusterTree-inst.h b/gtsam/inference/ClusterTree-inst.h index 2a5f0b4558..1a0b4470d9 100644 --- a/gtsam/inference/ClusterTree-inst.h +++ b/gtsam/inference/ClusterTree-inst.h @@ -114,20 +114,20 @@ ClusterTree::~ClusterTree() { for (auto&& root : roots_) { std::queue bfs_queue; - // first, move the root to the queue - bfs_queue.push(root); - root = nullptr; // now the root node is owned by the queue + + // first, steal the root and move it to the queue. This invalidates root + bfs_queue.push(std::move(root)); // for each node iterated, if its reference count is 1, it will be deleted while its children are still in the queue. // so that the recursive deletion will not happen. while (!bfs_queue.empty()) { - // move the ownership of the front node from the queue to the current variable - auto node = bfs_queue.front(); + // move the ownership of the front node from the queue to the current variable, invalidating the sharedClique at the front of the queue + auto node = std::move(bfs_queue.front()); bfs_queue.pop(); // add the children of the current node to the queue, so that the queue will also own the children nodes. for (auto child : node->children) { - bfs_queue.push(child); + bfs_queue.push(std::move(child)); } // leaving the scope of current will decrease the reference count of the current node by 1, and if the reference count is 0, // the node will be deleted. Because the children are in the queue, the deletion of the node will not trigger a recursive // deletion of the children. diff --git a/gtsam/inference/ClusterTree.h b/gtsam/inference/ClusterTree.h index 43859f3d30..f81029ba5b 100644 --- a/gtsam/inference/ClusterTree.h +++ b/gtsam/inference/ClusterTree.h @@ -166,9 +166,9 @@ class ClusterTree { /// @} + protected: ~ClusterTree(); - protected: /// @name Details /// @{ diff --git a/gtsam/inference/EliminationTree-inst.h b/gtsam/inference/EliminationTree-inst.h index 16449d0f4b..3ab0e9ce48 100644 --- a/gtsam/inference/EliminationTree-inst.h +++ b/gtsam/inference/EliminationTree-inst.h @@ -197,20 +197,19 @@ namespace gtsam { for (auto&& root : roots_) { std::queue bfs_queue; - // first, move the root to the queue - bfs_queue.push(root); - root = nullptr; // now the root node is owned by the queue + // first, steal the root and move it to the queue. This invalidates root + bfs_queue.push(std::move(root)); // for each node iterated, if its reference count is 1, it will be deleted while its children are still in the queue. // so that the recursive deletion will not happen. while (!bfs_queue.empty()) { - // move the ownership of the front node from the queue to the current variable - auto node = bfs_queue.front(); + // move the ownership of the front node from the queue to the current variable, invalidating the sharedClique at the front of the queue + auto node = std::move(bfs_queue.front()); bfs_queue.pop(); // add the children of the current node to the queue, so that the queue will also own the children nodes. for (auto&& child : node->children) { - bfs_queue.push(child); + bfs_queue.push(std::move(child)); } // leaving the scope of current will decrease the reference count of the current node by 1, and if the reference count is 0, // the node will be deleted. Because the children are in the queue, the deletion of the node will not trigger a recursive // deletion of the children. diff --git a/gtsam/inference/EliminationTree.h b/gtsam/inference/EliminationTree.h index 724c686083..557be16750 100644 --- a/gtsam/inference/EliminationTree.h +++ b/gtsam/inference/EliminationTree.h @@ -117,8 +117,10 @@ namespace gtsam { /// @} - public: + protected: ~EliminationTree(); + + public: /// @name Standard Interface /// @{