diff --git a/include/libsemigroups/schreier-sims.hpp b/include/libsemigroups/schreier-sims.hpp index b6662900b..c53bf29cb 100644 --- a/include/libsemigroups/schreier-sims.hpp +++ b/include/libsemigroups/schreier-sims.hpp @@ -295,7 +295,7 @@ namespace libsemigroups { //! \brief Default move constructor. //! //! Default move constructor. - SchreierSims(SchreierSims&&) = default; + SchreierSims(SchreierSims&&); //! \brief Default copy constructor. //! @@ -306,6 +306,7 @@ namespace libsemigroups { //! \brief Default move assignment. //! //! Default move assignment. + // TODO fix in the same way as the move constructor above SchreierSims& operator=(SchreierSims&&) = default; //! \brief Default copy assignment @@ -932,10 +933,9 @@ namespace libsemigroups { bool internal_equal_to(internal_const_element_type x, internal_const_element_type y) const - noexcept(noexcept( - EqualTo()(this->to_external_const(x), - this->to_external_const( - y)) && noexcept(this->to_external_const(x)))) { + noexcept(noexcept(EqualTo()(this->to_external_const(x), + this->to_external_const(y)) + && noexcept(this->to_external_const(x)))) { return EqualTo()(this->to_external_const(x), this->to_external_const(y)); } diff --git a/include/libsemigroups/schreier-sims.tpp b/include/libsemigroups/schreier-sims.tpp index 03dcb1d92..353cb8204 100644 --- a/include/libsemigroups/schreier-sims.tpp +++ b/include/libsemigroups/schreier-sims.tpp @@ -39,6 +39,8 @@ namespace libsemigroups { _tmp_element2(this->internal_copy(_one)), _transversal(), _inversal() { + fmt::print("Default constructing {} ! ! ! \n", fmt::ptr(this)); + _orbits_lookup.fill(false); init(); } @@ -54,10 +56,18 @@ namespace libsemigroups { template SchreierSims::~SchreierSims() { - clear(); - this->internal_free(_one); - this->internal_free(_tmp_element1); - this->internal_free(_tmp_element2); + fmt::print("Destructor for {} ! ! ! \n", fmt::ptr(this)); + if constexpr (std::is_pointer_v) { + if (_one != nullptr) { + // _one not being the nullptr indicates that this owns its data, and + // otherwise that it does not. This is required by the move + // constructors. + clear(); + this->internal_free(_tmp_element1); + this->internal_free(_tmp_element2); + this->internal_free(_one); + } + } } template @@ -67,7 +77,7 @@ namespace libsemigroups { _base_size(that._base_size), _domain(that._domain), _finished(that._finished), - _one(this->internal_copy(that._one)), + _one(this->to_internal(One()(N))), _orbits(that._orbits), _orbits_lookup(that._orbits_lookup), _strong_gens(), @@ -75,12 +85,41 @@ namespace libsemigroups { _tmp_element2(this->internal_copy(_one)), _transversal(), _inversal() { + fmt::print("Copy constructor from {} into {} ! ! ! \n", + fmt::ptr(&that), + fmt::ptr(this)); init_strong_gens_traversal_inversal(that); } + template + SchreierSims::SchreierSims(SchreierSims&& that) + : _base(std::move(that._base)), + _base_size(std::move(that._base_size)), + _domain(std::move(that._domain)), + _finished(std::move(that._finished)), + _one(std::move(that._one)), + _orbits(std::move(that._orbits)), + _orbits_lookup(std::move(that._orbits_lookup)), + _strong_gens(std::move(that._strong_gens)), + _tmp_element1(std::move(that._tmp_element1)), + _tmp_element2(std::move(that._tmp_element2)), + _transversal(std::move(that._transversal)), + _inversal(std::move(that._inversal)) { + if constexpr (std::is_pointer_v) { + that._one = nullptr; + } + + fmt::print("Move constructor from {} into {} ! ! ! \n", + fmt::ptr(&that), + fmt::ptr(this)); + } + template SchreierSims& SchreierSims::operator=(SchreierSims const& that) { + fmt::print("Copy assigning from {} into {} ! ! ! \n", + fmt::ptr(&that), + fmt::ptr(this)); _base = that._base; _base_size = that._base_size; _domain = that._domain; diff --git a/tests/test-schreier-sims.cpp b/tests/test-schreier-sims.cpp index 95924c407..e66dbb174 100644 --- a/tests/test-schreier-sims.cpp +++ b/tests/test-schreier-sims.cpp @@ -4422,8 +4422,8 @@ namespace libsemigroups { "[quick][schreier-sims][copy constructor]") { auto rg = ReportGuard(false); using Perm = Perm<0, uint8_t>; - SchreierSims<255, uint8_t, Perm> S1; - S1.add_generator(Perm( + SchreierSims<255, uint8_t, Perm>* S1 = new SchreierSims<255, uint8_t, Perm>(); + S1->add_generator(Perm( {1, 0, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, @@ -4443,7 +4443,7 @@ namespace libsemigroups { 224, 225, 226, 227, 228, 229, 230, 231, 232, 233, 234, 235, 236, 237, 238, 239, 240, 241, 242, 243, 244, 245, 246, 247, 248, 249, 250, 251, 252, 253, 254})); - S1.add_generator(Perm( + S1->add_generator(Perm( {1, 2, 3, 4, 0, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, @@ -4464,11 +4464,13 @@ namespace libsemigroups { 238, 239, 240, 241, 242, 243, 244, 245, 246, 247, 248, 249, 250, 251, 252, 253, 254})); - SchreierSims<255, uint8_t, Perm> S2(S1); - REQUIRE(&S1 != &S2); - REQUIRE(S1.number_of_generators() == S2.number_of_generators()); - REQUIRE(S1.current_size() == S2.current_size()); - REQUIRE(S1.finished() == S2.finished()); + SchreierSims<255, uint8_t, Perm>* S2 = new SchreierSims<255, uint8_t, Perm>(*S1); + delete S1; + // REQUIRE(&S1 != &S2); + // REQUIRE(S1->number_of_generators() == S2.number_of_generators()); + // REQUIRE(S1->current_size() == S2.current_size()); + // REQUIRE(S1->finished() == S2.finished()); + REQUIRE(S2->size() == 120); } } // namespace libsemigroups