Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Added virtual destructor and re-formatted constructors #870

Merged
merged 1 commit into from
Sep 7, 2021

Conversation

dellaert
Copy link
Member

@dellaert dellaert commented Sep 6, 2021

I saw a warning (output below) when I did make check on the latest develop, so I fixed it by adding a virtual constructor.

Consolidate compiler generated dependencies of target testFactorTesting
[ 65%] Building CXX object gtsam/nonlinear/tests/CMakeFiles/testFactorTesting.dir/testFactorTesting.cpp.o
In file included from /Users/dellaert/git/gtsam/gtsam/nonlinear/tests/testSerializationNonlinear.cpp:19:
In file included from /Users/dellaert/git/gtsam/gtsam/nonlinear/Values.h:27:
In file included from /Users/dellaert/git/gtsam/gtsam/base/GenericValue.h:22:
In file included from /Users/dellaert/git/gtsam/gtsam/base/Manifold.h:22:
In file included from /Users/dellaert/git/gtsam/gtsam/base/Matrix.h:28:
In file included from /Users/dellaert/git/gtsam/gtsam/base/Vector.h:267:
In file included from /opt/homebrew/include/boost/serialization/nvp.hpp:35:
In file included from /opt/homebrew/include/boost/serialization/split_free.hpp:22:
In file included from /opt/homebrew/include/boost/serialization/serialization.hpp:43:
/opt/homebrew/include/boost/serialization/access.hpp:123:9: warning: delete called on non-final 'gtsam::BayesTreeCliqueBase<gtsam::ISAM2Clique, gtsam::GaussianFactorGraph>' that has virtual functions but non-virtual destructor [-Wdelete-non-abstract-non-virtual-dtor]
        delete const_cast<T *>(t);
        ^
/opt/homebrew/include/boost/serialization/extended_type_info_typeid.hpp:135:39: note: in instantiation of function template specialization 'boost::serialization::access::destroy<gtsam::BayesTreeCliqueBase<gtsam::ISAM2Clique, gtsam::GaussianFactorGraph>>' requested here
        boost::serialization::access::destroy(
                                      ^
/opt/homebrew/include/boost/serialization/singleton.hpp:147:5: note: in instantiation of member function 'boost::serialization::extended_type_info_typeid<gtsam::BayesTreeCliqueBase<gtsam::ISAM2Clique, gtsam::GaussianFactorGraph>>::destroy' requested here
    singleton_wrapper(){
    ^
/opt/homebrew/include/boost/serialization/singleton.hpp:171:47: note: in instantiation of member function 'boost::serialization::detail::singleton_wrapper<boost::serialization::extended_type_info_typeid<gtsam::BayesTreeCliqueBase<gtsam::ISAM2Clique, gtsam::GaussianFactorGraph>>>::singleton_wrapper' requested here
        static detail::singleton_wrapper< T > t;
                                              ^
/opt/homebrew/include/boost/serialization/singleton.hpp:196:16: note: in instantiation of member function 'boost::serialization::singleton<boost::serialization::extended_type_info_typeid<gtsam::BayesTreeCliqueBase<gtsam::ISAM2Clique, gtsam::GaussianFactorGraph>>>::get_instance' requested here
        return get_instance();
               ^
/opt/homebrew/include/boost/serialization/void_cast.hpp:193:49: note: in instantiation of member function 'boost::serialization::singleton<boost::serialization::extended_type_info_typeid<gtsam::BayesTreeCliqueBase<gtsam::ISAM2Clique, gtsam::GaussianFactorGraph>>>::get_const_instance' requested here
        & type_info_implementation<Base>::type::get_const_instance(),
                                                ^
/opt/homebrew/include/boost/serialization/singleton.hpp:147:5: note: (skipping 185 contexts in backtrace; use -ftemplate-backtrace-limit=0 to see all)
    singleton_wrapper(){
    ^
/opt/homebrew/include/boost/archive/detail/common_oarchive.hpp:71:18: note: in instantiation of function template specialization 'boost::archive::save<boost::archive::binary_oarchive, const gtsam::ISAM2>' requested here
        archive::save(* this->This(), t);
                 ^
/opt/homebrew/include/boost/archive/basic_binary_oarchive.hpp:80:37: note: in instantiation of function template specialization 'boost::archive::detail::common_oarchive<boost::archive::binary_oarchive>::save_override<const gtsam::ISAM2>' requested here
      this->detail_common_oarchive::save_override(t);
                                    ^
/opt/homebrew/include/boost/archive/binary_oarchive_impl.hpp:59:47: note: in instantiation of function template specialization 'boost::archive::basic_binary_oarchive<boost::archive::binary_oarchive>::save_override<gtsam::ISAM2>' requested here
        this->basic_binary_oarchive<Archive>::save_override(t);
                                              ^
/opt/homebrew/include/boost/archive/detail/interface_oarchive.hpp:70:23: note: in instantiation of function template specialization 'boost::archive::binary_oarchive_impl<boost::archive::binary_oarchive, char, std::__1::char_traits<char>>::save_override<const gtsam::ISAM2>' requested here
        this->This()->save_override(t);
                      ^
/Users/dellaert/git/gtsam/gtsam/nonlinear/tests/testSerializationNonlinear.cpp:136:19: note: in instantiation of function template specialization 'boost::archive::detail::interface_oarchive<boost::archive::binary_oarchive>::operator<<<gtsam::ISAM2>' requested here
    outputArchive << solver;
                  ^

@dellaert dellaert added the cleanup Help clean up old/obsolete aspects of GTSAM label Sep 6, 2021
@dellaert dellaert requested a review from yetongumich September 6, 2021 15:28
@dellaert dellaert self-assigned this Sep 6, 2021
@varunagrawal
Copy link
Collaborator

You mean virtual destructor? This was on my radar for after ICRA since it only shows up on debug builds.

@varunagrawal varunagrawal changed the title Added virtual constructor and re-formatted constructors Added virtual destructor and re-formatted constructors Sep 6, 2021
Copy link
Collaborator

@varunagrawal varunagrawal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dellaert dellaert merged commit 59d9027 into develop Sep 7, 2021
@dellaert dellaert deleted the fix/constructor_warning branch September 7, 2021 01:41
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
cleanup Help clean up old/obsolete aspects of GTSAM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants