Skip to content

Commit

Permalink
Fix weirdness
Browse files Browse the repository at this point in the history
  • Loading branch information
@james-d-mitchell
james-d-mitchell committed Nov 18, 2024
1 parent 755df11 commit cc9f601
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 18 deletions.
10 changes: 5 additions & 5 deletions include/libsemigroups/schreier-sims.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ namespace libsemigroups {
//! \brief Default move constructor.
//!
//! Default move constructor.
SchreierSims(SchreierSims&&) = default;
SchreierSims(SchreierSims&&);

//! \brief Default copy constructor.
//!
Expand All @@ -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
Expand Down Expand Up @@ -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));
}

Expand Down
49 changes: 44 additions & 5 deletions include/libsemigroups/schreier-sims.tpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand All @@ -54,10 +56,18 @@ namespace libsemigroups {

template <size_t N, typename Point, typename Element, typename Traits>
SchreierSims<N, Point, Element, Traits>::~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<internal_element_type>) {
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 <size_t N, typename Point, typename Element, typename Traits>
Expand All @@ -67,20 +77,49 @@ 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(),
_tmp_element1(this->internal_copy(_one)),
_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 <size_t N, typename Point, typename Element, typename Traits>
SchreierSims<N, Point, Element, Traits>::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<internal_element_type>) {
that._one = nullptr;
}

fmt::print("Move constructor from {} into {} ! ! ! \n",
fmt::ptr(&that),
fmt::ptr(this));
}

template <size_t N, typename Point, typename Element, typename Traits>
SchreierSims<N, Point, Element, Traits>&
SchreierSims<N, Point, Element, Traits>::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;
Expand Down
18 changes: 10 additions & 8 deletions tests/test-schreier-sims.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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

0 comments on commit cc9f601

Please sign in to comment.