Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

axes should provide strong exception guarantee for metadata #379

Open
HDembinski opened this issue Dec 24, 2022 · 10 comments · Fixed by #382
Open

axes should provide strong exception guarantee for metadata #379

HDembinski opened this issue Dec 24, 2022 · 10 comments · Fixed by #382

Comments

@HDembinski
Copy link
Collaborator

HDembinski commented Dec 24, 2022

template <class It, class = detail::requires_iterator<It>> variable(It begin, It end, metadata_type meta = {}, options_type options = {}, allocator_type alloc = {}) does not provide strong exception guarantee: if user writes variable(it_begin, it_end, std::move(str)) and constructor body throws then the value inside str is lost (the value was moved into parameter and then data member before the exception). So one can not write while(std::cin>>str>>...){ try{ variable(..., ..., str); ... }catch(...){} } and has to take on the burden of saving the str elsewhere (so the user do not have to type it again) when other constructor arguments might be illegal and have to be typed again.

Originally posted by @jhcarl0814 in #372 (comment)

@jhcarl0814
Copy link
Contributor

jhcarl0814 commented Dec 25, 2022

@HDembinski Here is an example of constructor rollback "std::moves from parameters to data members" on exception:

#include<type_traits>
#include<iostream>
#include<boost/core/uncaught_exceptions.hpp>

template<typename T>
struct remove_rvalue_reference
{
    using type = typename std::conditional_t<std::is_rvalue_reference<T>::value, std::remove_reference_t<T>, T>;
};
template<typename T>
using remove_rvalue_reference_t = typename remove_rvalue_reference<T>::type;

template<typename TSrc, typename TDst>
struct rollback_move_on_exception
{
    unsigned int count_;
    std::remove_reference_t<TSrc> *src_;
    std::remove_reference_t<TDst> *dst_;

    rollback_move_on_exception(std::remove_reference_t<TSrc> *src, std::remove_reference_t<TDst> *dst)
        : count_(boost::core::uncaught_exceptions()),
          src_(src),
          dst_(dst)
    {}

    ~rollback_move_on_exception()
    {
        if(count_ != boost::core::uncaught_exceptions())
        {
            *src_ = std::move(*dst_);
        }
    }
};
template<typename TSrc, typename TDst>
struct rollback_move_on_exception<TSrc &, TDst>
{
    rollback_move_on_exception(std::remove_reference_t<TSrc> *, std::remove_reference_t<TDst> *) {}
};

#define ROLLBACK_MOVE_ON_EXCEPTION(parameter_exp, member_exp) rollback_move_on_exception<remove_rvalue_reference_t<decltype(parameter_exp)>, decltype(member_exp)> s_guard(&parameter_exp, &member_exp)

struct c1_t
{
    std::string s_;
    std::string&get_s(){return s_;}
    int n_;

    template<typename Str>
    c1_t(Str &&s, bool thrw)
        : s_(std::forward<Str>(s)),
          n_([&]
            {
                ROLLBACK_MOVE_ON_EXCEPTION(s, get_s());
            //   ROLLBACK_MOVE_ON_EXCEPTION(s, s_);
              if(thrw) throw 1;
              return 0;
            }())
    {
        ROLLBACK_MOVE_ON_EXCEPTION(s, s_);
        if(thrw) throw 1;
    }
};

int main(int argc, char *argv[])
{
    {
        std::string s("abc");
        c1_t c1(s, false);
        std::cout << 1 << s << '\n';
    }
    {
        std::string s("abc");
        c1_t c1(std::move(s), false);
        std::cout << 2 << s << '\n';
    }
    {
        std::string s("abc");
        try
        {
            c1_t c1(s, true);
        }
        catch(...)
        {
        }
        std::cout << 3 << s << '\n';
    }
    {
        std::string s("abc");
        try
        {
            c1_t c1(std::move(s), true);
        }
        catch(...)
        {
        }
        std::cout << 4 << s << '\n';
    }
}
1abc
2
3abc
4abc

good:

  • types of bases/data_members don't have to be DefaultConstructible (useful in general but not relevant to axis::variable, its bases/data_members are already DefaultConstructible)
  • preserves the initialization order of bases/data_members

bad:

  • for each base/data_member A (that is move constructed):
    • for each base/data_member B that is declared after A and whose construction might throw:
      • programmer has to change to [&]{ ... }() to initialize B and repeat that macro for A in the lambda
    • programmer has to repeat that macro for A in the constructor body (even if the constructor body is empty, because it's too easy to forget to add it in the future)

not possible to cover:

  • when It begin, It end gives prvalues (e.g. std::istream_iterator)

too troublesome to cover:

  • when It begin, It end gives xvalues (e.g. std::move_iterator)

modified version of include\boost\histogram\axis\variable.hpp:

// Copyright 2015-2018 Hans Dembinski
//
// Distributed under the Boost Software License, Version 1.0.
// (See accompanying file LICENSE_1_0.txt
// or copy at http://www.boost.org/LICENSE_1_0.txt)

#ifndef BOOST_HISTOGRAM_AXIS_VARIABLE_HPP
#define BOOST_HISTOGRAM_AXIS_VARIABLE_HPP

#include <algorithm>
#include <boost/core/nvp.hpp>
#include <boost/histogram/axis/interval_view.hpp>
#include <boost/histogram/axis/iterator.hpp>
#include <boost/histogram/axis/metadata_base.hpp>
#include <boost/histogram/axis/option.hpp>
#include <boost/histogram/detail/convert_integer.hpp>
#include <boost/histogram/detail/detect.hpp>
#include <boost/histogram/detail/eytzinger_search.hpp>
#include <boost/histogram/detail/limits.hpp>
#include <boost/histogram/detail/relaxed_equal.hpp>
#include <boost/histogram/detail/replace_type.hpp>
#include <boost/histogram/fwd.hpp>
#include <boost/throw_exception.hpp>
#include <cassert>
#include <cmath>
#include <limits>
#include <memory>
#include <stdexcept>
#include <string>
#include <type_traits>
#include <utility>
#include <vector>

#include <boost/core/uncaught_exceptions.hpp>

template <typename T>
struct remove_rvalue_reference {
  using type = typename std::conditional_t<std::is_rvalue_reference<T>::value,
                                           std::remove_reference_t<T>, T>;
};
template <typename T>
using remove_rvalue_reference_t = typename remove_rvalue_reference<T>::type;

template <typename TSrc, typename TDst>
struct rollback_move_on_exception {
  unsigned int count_;
  std::remove_reference_t<TSrc>* src_;
  std::remove_reference_t<TDst>* dst_;

  rollback_move_on_exception(std::remove_reference_t<TSrc>* src,
                             std::remove_reference_t<TDst>* dst)
      : count_(boost::core::uncaught_exceptions()), src_(src), dst_(dst) {}

  ~rollback_move_on_exception() {
    if (count_ != boost::core::uncaught_exceptions()) { *src_ = std::move(*dst_); }
  }
};
template <typename TSrc, typename TDst>
struct rollback_move_on_exception<TSrc&, TDst> {
  rollback_move_on_exception(std::remove_reference_t<TSrc>*,
                             std::remove_reference_t<TDst>*) {}
};

#define ROLLBACK_MOVE_ON_EXCEPTION(parameter_exp, member_exp)                    \
  rollback_move_on_exception<remove_rvalue_reference_t<decltype(parameter_exp)>, \
                             decltype(member_exp)>                               \
      s_guard(&parameter_exp, &member_exp)

namespace boost {
namespace histogram {
namespace axis {

/** Axis for non-equidistant bins on the real line.

  Binning is a O(log(N)) operation. If speed matters and the problem domain
  allows it, prefer a regular axis, possibly with a transform.

  If the axis has an overflow bin (the default), a value on the upper edge of the last
  bin is put in the overflow bin. The axis range represents a semi-open interval.

  If the overflow bin is deactivated, then a value on the upper edge of the last bin is
  still counted towards the last bin. The axis range represents a closed interval. This
  is the desired behavior for random numbers drawn from a bounded interval, which is
  usually closed.

  @tparam Value     input value type, must be floating point.
  @tparam MetaData  type to store meta data.
  @tparam Options   see boost::histogram::axis::option.
  @tparam Allocator allocator to use for dynamic memory management.
*/
template <class Value, class MetaData, class Options, class Allocator>
class variable : public iterator_mixin<variable<Value, MetaData, Options, Allocator>>,
                 public metadata_base_t<MetaData> {
  // these must be private, so that they are not automatically inherited
  using value_type = Value;
  using metadata_base = metadata_base_t<MetaData>;
  using metadata_type = typename metadata_base::metadata_type;
  using options_type =
      detail::replace_default<Options, decltype(option::underflow | option::overflow)>;
  using allocator_type = Allocator;
  using vector_type = std::vector<Value, allocator_type>;

public:
  constexpr variable() = default;
  explicit variable(allocator_type alloc) : vec_(alloc) {}

  /** Construct from iterator range of bin edges.

     @param begin   begin of edge sequence.
     @param end     end of edge sequence.
     @param meta    description of the axis (optional).
     @param options see boost::histogram::axis::option (optional).
     @param alloc   allocator instance to use (optional).
   */
  template <class It, class Metadata = metadata_type, class = detail::requires_iterator<It>>
  variable(It begin, It end, Metadata&& meta = {}, options_type options = {},
           allocator_type alloc = {})
      : metadata_base(std::forward<Metadata>(meta))
      , vec_([&] {
        ROLLBACK_MOVE_ON_EXCEPTION(meta, this->metadata());
        if (std::distance(begin, end) < 2)
          BOOST_THROW_EXCEPTION(std::invalid_argument("bins > 0 required"));

        vector_type vec(std::move(alloc));

        vec.reserve(std::distance(begin, end));
        vec.emplace_back(*begin++);
        bool strictly_ascending = true;
        for (; begin != end; ++begin) {
          strictly_ascending &= vec.back() < *begin;
          vec.emplace_back(*begin);
        }
        if (!strictly_ascending)
          BOOST_THROW_EXCEPTION(
              std::invalid_argument("input sequence must be strictly ascending"));
        return vec;
      }())
      , eytzinger_layout_and_eytzinger_binary_search_([&] {
        ROLLBACK_MOVE_ON_EXCEPTION(meta, this->metadata());
        return decltype(eytzinger_layout_and_eytzinger_binary_search_)(vec_);
      }()) {
    ROLLBACK_MOVE_ON_EXCEPTION(meta, this->metadata());
    // static_asserts were moved here from class scope to satisfy deduction in gcc>=11
    static_assert(
        std::is_floating_point<value_type>::value,
        "current version of variable axis requires floating point type; "
        "if you need a variable axis with an integral type, please submit an issue");
    static_assert((!options.test(option::circular) && !options.test(option::growth)) ||
                      (options.test(option::circular) ^ options.test(option::growth)),
                  "circular and growth options are mutually exclusive");
  }

  // kept for backward compatibility; requires_allocator is a workaround for deduction
  // guides in gcc>=11
  template <class It, class Metadata, class A, class = detail::requires_iterator<It>,
            class = detail::requires_allocator<A>>
  variable(It begin, It end, Metadata&& meta, A alloc)
      : variable(begin, end, std::forward<Metadata>(meta), {}, std::move(alloc)) {}

  /** Construct variable axis from iterable range of bin edges.

     @param iterable iterable range of bin edges.
     @param meta     description of the axis (optional).
     @param options  see boost::histogram::axis::option (optional).
     @param alloc    allocator instance to use (optional).
   */
  template <class U, class Metadata = metadata_type, class = detail::requires_iterable<U>>
  variable(const U& iterable, Metadata&& meta = {}, options_type options = {},
           allocator_type alloc = {})
      : variable(std::begin(iterable), std::end(iterable), std::forward<Metadata>(meta),
                 options, std::move(alloc)) {}

  // kept for backward compatibility; requires_allocator is a workaround for deduction
  // guides in gcc>=11
  template <class U, class Metadata, class A, class = detail::requires_iterable<U>,
            class = detail::requires_allocator<A>>
  variable(const U& iterable, Metadata&& meta, A alloc)
      : variable(std::begin(iterable), std::end(iterable), std::forward<Metadata>(meta),
                 {}, std::move(alloc)) {}

  /** Construct variable axis from initializer list of bin edges.

     @param list     `std::initializer_list` of bin edges.
     @param meta     description of the axis (optional).
     @param options  see boost::histogram::axis::option (optional).
     @param alloc    allocator instance to use (optional).
   */
  template <class U, class Metadata = metadata_type>
  variable(std::initializer_list<U> list, Metadata&& meta = {},
           options_type options = {}, allocator_type alloc = {})
      : variable(list.begin(), list.end(), std::forward<Metadata>(meta), options,
                 std::move(alloc)) {}

  // kept for backward compatibility; requires_allocator is a workaround for deduction
  // guides in gcc>=11
  template <class U, class Metadata, class A, class = detail::requires_allocator<A>>
  variable(std::initializer_list<U> list, Metadata&& meta, A alloc)
      : variable(list.begin(), list.end(), std::forward<Metadata>(meta), {},
                 std::move(alloc)) {}

  /// Constructor used by algorithm::reduce to shrink and rebin (not for users).
  variable(const variable& src, index_type begin, index_type end, unsigned merge)
      : metadata_base(src)
      , vec_([&] {
        assert((end - begin) % merge == 0);
        if (options_type::test(option::circular) && !(begin == 0 && end == src.size()))
          BOOST_THROW_EXCEPTION(std::invalid_argument("cannot shrink circular axis"));

        vector_type vec(src.get_allocator());
        vec.reserve((end - begin) / merge);
        const auto beg = src.vec_.begin();
        for (index_type i = begin; i <= end; i += merge) vec.emplace_back(*(beg + i));
        return vec;
      }())
      , eytzinger_layout_and_eytzinger_binary_search_(vec_) {}

  /// Return index for value argument.
  index_type index(value_type x) const noexcept {
    if (options_type::test(option::circular)) {
      const auto a = vec_[0];
      const auto b = vec_[size()];
      x -= std::floor((x - a) / (b - a)) * (b - a);
    }
    // upper edge of last bin is inclusive if overflow bin is not present
    if (!options_type::test(option::overflow) && x == vec_.back()) return size() - 1;
    return eytzinger_layout_and_eytzinger_binary_search_.index(x);
  }

  std::pair<index_type, index_type> update(value_type x) noexcept {
    const auto i = index(x);
    if (std::isfinite(x)) {
      if (0 <= i) {
        if (i < size()) return std::make_pair(i, 0);
        const auto d = value(size()) - value(size() - 0.5);
        x = std::nextafter(x, (std::numeric_limits<value_type>::max)());
        x = (std::max)(x, vec_.back() + d);
        vec_.push_back(x);
        eytzinger_layout_and_eytzinger_binary_search_.assign(vec_);
        return {i, -1};
      }
      const auto d = value(0.5) - value(0);
      x = (std::min)(x, value(0) - d);
      vec_.insert(vec_.begin(), x);
      eytzinger_layout_and_eytzinger_binary_search_.assign(vec_);
      return {0, -i};
    }
    return {x < 0 ? -1 : size(), 0};
  }

  /// Return value for fractional index argument.
  value_type value(real_index_type i) const noexcept {
    if (options_type::test(option::circular)) {
      auto shift = std::floor(i / size());
      i -= shift * size();
      double z;
      const auto k = static_cast<index_type>(std::modf(i, &z));
      const auto a = vec_[0];
      const auto b = vec_[size()];
      return (1.0 - z) * vec_[k] + z * vec_[k + 1] + shift * (b - a);
    }
    if (i < 0) return detail::lowest<value_type>();
    if (i == size()) return vec_.back();
    if (i > size()) return detail::highest<value_type>();
    const auto k = static_cast<index_type>(i); // precond: i >= 0
    const real_index_type z = i - k;
    // check z == 0 needed to avoid returning nan when vec_[k + 1] is infinity
    return (1.0 - z) * vec_[k] + (z == 0 ? 0 : z * vec_[k + 1]);
  }

  /// Return bin for index argument.
  auto bin(index_type idx) const noexcept { return interval_view<variable>(*this, idx); }

  /// Returns the number of bins, without over- or underflow.
  index_type size() const noexcept { return static_cast<index_type>(vec_.size()) - 1; }

  /// Returns the options.
  static constexpr unsigned options() noexcept { return options_type::value; }

  template <class V, class M, class O, class A>
  bool operator==(const variable<V, M, O, A>& o) const noexcept {
    const auto& a = vec_;
    const auto& b = o.vec_;
    return std::equal(a.begin(), a.end(), b.begin(), b.end()) &&
           detail::relaxed_equal{}(this->metadata(), o.metadata());
  }

  template <class V, class M, class O, class A>
  bool operator!=(const variable<V, M, O, A>& o) const noexcept {
    return !operator==(o);
  }

  /// Return allocator instance.
  auto get_allocator() const { return vec_.get_allocator(); }

  template <class Archive>
  void serialize(Archive& ar, unsigned /* version */) {
    ar& make_nvp("seq", vec_);
    ar& make_nvp("meta", this->metadata());
  }

private:
  vector_type vec_;
  boost::histogram::detail::eytzinger_layout_and_eytzinger_binary_search_t<Value>
      eytzinger_layout_and_eytzinger_binary_search_;

  template <class V, class M, class O, class A>
  friend class variable;
};

#if __cpp_deduction_guides >= 201606

template <class T>
variable(std::initializer_list<T>)
    -> variable<detail::convert_integer<T, double>, null_type>;

template <class T, class M>
variable(std::initializer_list<T>, M)
    -> variable<detail::convert_integer<T, double>,
                detail::replace_type<std::decay_t<M>, const char*, std::string>>;

template <class T, class M, unsigned B>
variable(std::initializer_list<T>, M, const option::bitset<B>&)
    -> variable<detail::convert_integer<T, double>,
                detail::replace_type<std::decay_t<M>, const char*, std::string>,
                option::bitset<B>>;

template <class Iterable, class = detail::requires_iterable<Iterable>>
variable(Iterable) -> variable<
    detail::convert_integer<
        std::decay_t<decltype(*std::begin(std::declval<Iterable&>()))>, double>,
    null_type>;

template <class Iterable, class M>
variable(Iterable, M) -> variable<
    detail::convert_integer<
        std::decay_t<decltype(*std::begin(std::declval<Iterable&>()))>, double>,
    detail::replace_type<std::decay_t<M>, const char*, std::string>>;

template <class Iterable, class M, unsigned B>
variable(Iterable, M, const option::bitset<B>&) -> variable<
    detail::convert_integer<
        std::decay_t<decltype(*std::begin(std::declval<Iterable&>()))>, double>,
    detail::replace_type<std::decay_t<M>, const char*, std::string>, option::bitset<B>>;

#endif

} // namespace axis
} // namespace histogram
} // namespace boost

#endif

@HDembinski
Copy link
Collaborator Author

The rollback_move_on_exception guard is cool, but maybe we fix it without using that.

@HDembinski
Copy link
Collaborator Author

HDembinski commented Dec 26, 2022

I gave this more thought and I don't want to fix this. I don't like that we have to template the Metadata parameter for this to work. There is a compilation cost associated to all the template instantiations and we try to keep that minimal, too.

Instead, the caveat of the design will be documented in the docstring of the ctor.

@HDembinski
Copy link
Collaborator Author

We don't have to worry about moving from the iterators, because the iterators will only ever by simple arithmetic types, were moves do not give you an advantage.

@HDembinski
Copy link
Collaborator Author

HDembinski commented Dec 26, 2022

@jhcarl0814 What do you think about this design, which is based on your rollback_move_on_exception idea. It delays the move until the constructor has completed and only performs it if there was no exception. I have to use a helper type in the public API delayed_forward, but this also has an advantage: the implictly generated deduction guide is now no longer valid, and now even gcc picks up the custom deduction guide. The implementation also delays copies until the axis ctor succeeded.

The change of initialization order is a not a problem, since initialization order is an implementation detail. We turn an move/copy-ctor into default ctor + move/copy-assign. That's ok, the MetaData type must be assignable and default constructible.

The ultimate conclusion of Hyrum's law is that you cannot change anything ever. Boost does not follow that, because we want to keep some room for improvements. We are not the C++ stdlib. That being said, any change to the API has to be done with care and must not break existing code (without a deprecation period and prior warning).

Godbolt

#include <iostream>
#include <cassert>
#include <stdexcept>
#include <boost/core/lightweight_test.hpp>

struct meta {
    void msg(const char* msg) {
        std::cerr << msg << std::endl;
    }

    meta() { msg("Default ctor"); }
    meta(int v) : value_{v} { msg("Init ctor"); }
    meta(const meta& o) : value_{o.value_} { msg("Copy ctor"); }
    meta( meta&& o) noexcept {
        msg("Move ctor"); 
        value_ = o.value_;
        o.value_ = 0;
    }
    meta& operator=(meta&& o) noexcept {
        msg("Move assign");
        value_ = o.value_;
        o.value_ = 0;
        return *this;
    }
    meta& operator=(const meta& o) { 
        msg("Copy assign");
        value_ = o.value_;
        return *this;
    }

    ~meta() {
        if (value_)
           msg("Destroy");
    }

    int value_ = 0;
};

template <class T>
struct delayed_forward {
    delayed_forward() = default;
    delayed_forward(T&& t) : src_{&t} {}
    delayed_forward(const T& t) : src_{const_cast<T*>(&t)}, move_{false} {}

    void operator()(T& dst) {
        if (move_)
            dst = std::move(*src_);
        else
            dst = *src_;
    }

    T* src_ = nullptr;
    bool move_ = true;
};

template <class Meta>
struct axis {
    using metadata_type = Meta;

    axis(unsigned n, delayed_forward<metadata_type> meta = {}) {
        if (n == 0)
            throw std::invalid_argument("bad argument");
        meta(meta_); // must come after potential exceptions
    }

private:
    metadata_type meta_;
};

// deduction guide
template <class M>
axis(unsigned, M) -> axis<M>;

int main() {

    {   // move with throw
        meta m = 1;
        try {
            axis(0, std::move(m));
        } catch(...) {
            BOOST_TEST_EQ(m.value_, 1);
        }
    }

    {   // move without throw
        meta m = 1;
        axis(1, std::move(m));
        BOOST_TEST_EQ(m.value_, 0);
    }

    {   // copy with throw
        meta m = 1;
        try {
            axis(0, m);
        } catch(...) {
            BOOST_TEST_EQ(m.value_, 1);
        }
    }

    {   // copy without throw
        meta m = 1;
        axis(1, m);
        BOOST_TEST_EQ(m.value_, 1);
    }

    // deduction guide
    axis(1, meta{1});

    return boost::report_errors();
}

@HDembinski HDembinski changed the title axis::variable and other axes which can throw should provide strong exception guarantee axes should provide strong exception guarantee for metadata Dec 26, 2022
@jhcarl0814
Copy link
Contributor

@HDembinski "moving from the iterators" is okay, but "moving from the things the iterators point to" is-a side-effect:

  • ... = *it/vec_.emplace_back(*it) has a side effect if It is a specialization of std::move_iterator, you might want to save *it's address and move the content back to it when leaving the constructor via exception (storing a growing list of pointers might also throw an exception)
  • it++ has a side effect if It is a specialization of std::istream_iterator, you might want to startTransaction before reading from the stream, commitTransaction when leaving the constructor normally, rollbackTransaction when leaving the constructor via exception :)

The delayed_forward solution is cool! It also eliminates the need to update constructors' template parameter lists (and ... default template arguments). Here's a version that added a check against delayed_forward default-init case.

@HDembinski
Copy link
Collaborator Author

HDembinski commented Dec 28, 2022

@jhcarl0814

  • Ok, I see the point that std::move_iterator could potentially have side effects. For plain arithmetic types, moving typically does the same as copying, since otherwise a move would be more costly than a copy, but that's not guaranteed.
  • std::istream_iterator is not allowed, only forward_iterators are allowed. In C++14, we cannot catch this at compile-time with concepts, but it is now properly documented.

The delayed_forward solution is cool! It also eliminates the need to update constructors' template parameter lists (and ... default template arguments). Here's a version that added a check against delayed_forward default-init case.

Yes that's good. It would be great if we could enforce at compile-time that the developer calls void operator()(T& dst) exactly once.

@HDembinski
Copy link
Collaborator Author

I am going to reopen this issue, since it would be good to address this as best as possible. The perfect is the enemy of the good.

@HDembinski HDembinski reopened this Dec 28, 2022
@jhcarl0814
Copy link
Contributor

jhcarl0814 commented Dec 29, 2022

@HDembinski Boost.Multiprecision doc says that number (of which boost::multiprecision::float128 is a specialization) uses move semantics, so moving from a boost::multiprecision::float128 may have side effect and those move operations need to be delayed or rollbacked in generic (general) when Value is unknown.
But boost::multiprecision::float128 does not satisfy std::is_floating_point/boost::is_floating_point (they are used for type categories), maybe should use std::numeric_limits<T>::is_specialized && !std::numeric_limits<T>::is_integer in variable instead?


One way to force constructor author invoke the move logic only once is to let constructor author pass the function body to delayed_forward and let delayed_forward execute the function body and then move logic. Since constructor author is unlikely to write function body twice (which would be an obvious mistake), the move logic is also unlikely to be executed twice.

example

#include <iostream>
#include <cassert>
#include <stdexcept>
#include <boost/core/lightweight_test.hpp>
// #include<boost/core/uncaught_exceptions.hpp>

struct meta {
    void msg(const char* msg) {
        std::cerr << msg << std::endl;
    }

    meta() { msg("Default ctor"); }
    meta(int v) : value_{v} { msg("Init ctor"); }
    meta(const meta& o) : value_{o.value_} { msg("Copy ctor"); }
    meta( meta&& o) noexcept {
        msg("Move ctor"); 
        value_ = o.value_;
        o.value_ = 0;
    }
    meta& operator=(meta&& o) noexcept {
        msg("Move assign");
        value_ = o.value_;
        o.value_ = 0;
        return *this;
    }
    meta& operator=(const meta& o) { 
        msg("Copy assign");
        value_ = o.value_;
        return *this;
    }

    ~meta() {
        if (value_)
           msg("Destroy");
    }

    int value_ = 0;
};

template<typename T>
T forward_decltype_auto(std::remove_reference_t<T> &arg) //https://stackoverflow.com/a/57440814/8343353
{
    return std::forward<T>(arg);
}

template <class T>
struct delayed_forward {
    delayed_forward() = default;
    delayed_forward(T&& t) : src_{&t} {}
    delayed_forward(const T& t) : src_{const_cast<T*>(&t)}, move_{false} {}

    template<typename F>
    decltype(auto) function_body(T& dst, F&&f)
    {
        // struct guard_t
        // {
        //     delayed_forward* this_;
        //     T& dst_;
        //     unsigned int count_ = boost::core::uncaught_exceptions();
        //     ~guard_t()
        //     {
        //         if(count_ == boost::core::uncaught_exceptions())
        //         {
        //             if(this_->src_ != nullptr)
        //             {
        //                 if (this_->move_)
        //                     dst_ = std::move(*this_->src_);
        //                 else
        //                     dst_ = *this_->src_;
        //             }
        //         }
        //     }
        // }guard{this,dst};
        // return std::forward<F>(f);

        try
        {
            decltype(auto)result = std::forward<F>(f);
            if(src_ != nullptr)
            {
                if (move_)
                    dst = std::move(*src_);
                else
                    dst = *src_;
            }
            return forward_decltype_auto<decltype(result)>(result);
        }
        catch(...)
        {
            throw;
        }
    }

    T* src_ = nullptr;
    bool move_ = true;
};

template <class Meta = meta>
struct axis {
    using metadata_type = Meta;

    axis(unsigned n, delayed_forward<metadata_type> meta = {}) {
        meta.function_body(meta_,[&]
        {
            if (n == 0)
                throw std::invalid_argument("bad argument");
        });
    }

private:
    metadata_type meta_;
};

// deduction guide
template <class M>
axis(unsigned, M) -> axis<M>;

int main() {

    {
        try {
            axis(0);
        } catch(...) {
        }
    }
    std::cerr<<'\n';
    
    {
        axis(1);
    }
    std::cerr<<'\n';

    {   // move with throw
        meta m = 1;
        try {
            axis(0, std::move(m));
        } catch(...) {
            BOOST_TEST_EQ(m.value_, 1);
        }
    }
    std::cerr<<'\n';

    {   // move without throw
        meta m = 1;
        axis(1, std::move(m));
        BOOST_TEST_EQ(m.value_, 0);
    }
    std::cerr<<'\n';

    {   // copy with throw
        meta m = 1;
        try {
            axis(0, m);
        } catch(...) {
            BOOST_TEST_EQ(m.value_, 1);
        }
    }
    std::cerr<<'\n';

    {   // copy without throw
        meta m = 1;
        axis(1, m);
        BOOST_TEST_EQ(m.value_, 1);
    }
    std::cerr<<'\n';

    // deduction guide
    axis(1, meta{1});

    return boost::report_errors();
}

@HDembinski
Copy link
Collaborator Author

@jhcarl0814 Just to note, I am currently discussing this topic on the boost-dev mailing list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants