diff --git a/dune/subgrid/common/variant.hh b/dune/subgrid/common/variant.hh index e2132ea6c66de8266669eb87f8b9a44fe6143837..4b23e38f1abed006f976f37556643bb4a8f05b18 100644 --- a/dune/subgrid/common/variant.hh +++ b/dune/subgrid/common/variant.hh @@ -11,6 +11,9 @@ namespace Dune { namespace Std { namespace Impl { + // indicator value if something's not yet (or not any longer) valid + constexpr const auto invalidIndex = std::numeric_limits<size_t>::max(); + /* helper constructs to find position of a type T in a pack Ts... */ template <typename T, typename... Ts> struct index_in_pack; @@ -63,6 +66,9 @@ namespace Impl { const auto& get() const { return tp_; } + + void reset() {}; + private: Tp tp_; }; @@ -78,6 +84,8 @@ namespace Impl { ::new (&tp_) Tp(std::forward<Args>(args)...); } + TypeStorage_() = delete; + auto& get() { return *(tp_.ptr()); } @@ -85,12 +93,23 @@ namespace Impl { return *(tp_.ptr()); } + void reset() { + // Properly destruct the member: + tp_.ptr()->~Tp(); + // (the memory itself doesn't need to be free'd. This is done when the Buffer_ member gets destructed) + } + private: Buffer_<Tp> tp_; }; template<typename... T> - union variant_union_ {}; + union variant_union_ { + // dummy (this should never be called) + void resetByIndex(size_t) { + assert(false); + }; + }; template<typename Head_, typename... Tail_> union variant_union_<Head_, Tail_...> { @@ -124,11 +143,22 @@ namespace Impl { return tail_.getByIndex(std::integral_constant<size_t, N-1>()); } + void resetByIndex(size_t indexToReset) { + if (indexToReset == 0) { + head_.reset(); + return; + } + else { + tail_.resetByIndex(indexToReset-1); + } + } + template<typename Tp> - void set(Tp obj) { - Dune::Hybrid::ifElse(std::is_same<Tp, Head_>(), - [&](auto&& id) { head_=std::move(id(obj)); }, - [&](auto&& id) { return id(tail_).set(std::move(obj)); } + void set(Tp&& obj) { + using T = std::decay_t<Tp>; + Dune::Hybrid::ifElse(std::is_same<T, Head_>(), + [&](auto&& id) { id(head_)=std::forward<Tp>(obj); }, + [&](auto&& id) { return id(tail_).set(std::forward<Tp>(obj)); } ); } @@ -137,7 +167,7 @@ namespace Impl { } private: - TypeStorage_<Head_, std::is_literal_type<Head_>::value> head_; + TypeStorage_<Head_, std::is_trivial<Head_>::value> head_; variant_union_<Tail_...> tail_; }; @@ -145,7 +175,8 @@ namespace Impl { struct variant_{ constexpr variant_() : - unions_() {} + unions_(), + index_(invalidIndex) {} template<typename Tp> constexpr variant_(Tp obj) : @@ -155,6 +186,45 @@ namespace Impl { unions_.set(std::move(obj)); } + variant_(variant_&& other) { + unions_ = std::move(other.unions_); + index_ = other.index_; + other.index_ = invalidIndex; + } + + variant_(const variant_& other) { + index_ = other.index_; + + namespace H = Dune::Hybrid; + H::forEach(H::integralRange(std::integral_constant<size_t, size_>()), [&](auto i) { + if(i==index_) + unions_.set(other.get<i>()); + }); + } + + variant_& operator=(const variant_& other) { + if(index_ != invalidIndex) + unions_.resetByIndex(index_); + + index_ = other.index_; + + namespace H = Dune::Hybrid; + H::forEach(H::integralRange(std::integral_constant<size_t, size_>()), [&](auto i) { + if(i==index_) + unions_.set(other.get<i>()); + }); + return *this; + } + + variant_& operator=(variant_&& other) { + unions_ = std::move(other.unions_); + index_ = other.index_; + other.index_ = invalidIndex; + + return *this; + } + + template<typename Tp> auto& get() { constexpr size_t idx = index_in_pack<Tp, T...>::value; @@ -209,21 +279,25 @@ namespace Impl { template<size_t N> auto& get() { - if (index_ != N) + if (index_ != N || index_ == invalidIndex) DUNE_THROW(Dune::Exception, "Bad variant access."); return unions_.template getByIndex(std::integral_constant<size_t, N>()); } template<size_t N> const auto& get() const { - if (index_ != N) + if (index_ != N || index_ == invalidIndex) DUNE_THROW(Dune::Exception, "Bad variant access."); return unions_.template getByIndex(std::integral_constant<size_t, N>()); } template<typename Tp> constexpr Tp& operator=(Tp obj) { - unions_.set(std::move(obj)); constexpr auto index = index_in_pack<Tp, T...>::value; + // before setting a new object into the buffer, we have to destruct the old element + if(not (index_ == index or index_ == invalidIndex)) { + unions_.resetByIndex(index_); + } + unions_.set(std::move(obj)); index_=index; return unions_.getByIndex(std::integral_constant<size_t,index>()); } @@ -236,6 +310,12 @@ namespace Impl { return sizeof...(T); } + ~variant_() { + if (index_ != invalidIndex) { + unions_.resetByIndex(index_); + } + } + /* \brief Apply visitor to the active variant. * * visit assumes that the result of diff --git a/dune/subgrid/test/testvariant.cc b/dune/subgrid/test/testvariant.cc index ca0393f89e1c53ab03a65ce361ddeea4fe45d7bb..82e29c0f6edf06f9ffd5b200cd594a6beb1b7558 100644 --- a/dune/subgrid/test/testvariant.cc +++ b/dune/subgrid/test/testvariant.cc @@ -83,6 +83,33 @@ Dune::TestSuite testVariant() { suite.check(Std::visit(size, constv2)== 2, "Test const visit"); suite.check(Std::get_if<V2>(constv2) != nullptr, "Test const get_if"); + /// test copy and move construction/assignment + { + auto variant_copy_constructed = variant2; + // test if deep copy happened + auto getPtrToData = [&](const auto& vec) {return static_cast<const void*>(vec.data()); }; + suite.check(Std::visit(getPtrToData, variant_copy_constructed) != Std::visit(getPtrToData, variant2), "Check deep copy") << "Both vector copies point to same data"; + + auto variant_move_constructed = std::move(variant_copy_constructed); + // TODO: Add sensible test condition here. + // Testing if the pointer to the data is the same as before is probably not a good idea, + // as moving does not imply that the data really stays at the same place (though it probably + // does). + // At least if this compiles and runs we can be confident no double frees happened. + // + // First idea: Test if the state looks somewhat valid + suite.check(Std::holds_alternative<V2>(variant_move_constructed), "Check if move constructed variant holds the right type"); + + Std::variant<V, V2> variant_copy_assigned; + variant_copy_assigned = variant2; + // test if deep copy happened + suite.check(Std::visit(getPtrToData, variant_copy_assigned) != Std::visit(getPtrToData, variant2), "Check deep copy at operator=") << "Both vector copies point to same data"; + + Std::variant<V, V2> variant_move_assigned; + variant_move_assigned = std::move(variant_copy_assigned); + // TODO: Again, as above, find a better test for this. + suite.check(Std::holds_alternative<V2>(variant_move_assigned), "Check if move assigned variant holds the right type"); + } return suite; }