simpler variant

* simpler, now correct variant
* replaced reference_wrapper with pointer

include/boost/histogram/axis/variant.hpp

@@ -16,14 +16,12 @@
 #include <boost/histogram/detail/type_name.hpp>
 #include <boost/histogram/detail/variant.hpp>
 #include <boost/histogram/fwd.hpp>
-#include <boost/mp11/bind.hpp>
 #include <boost/mp11/function.hpp>
 #include <boost/mp11/list.hpp>
+#include <boost/mp11/utility.hpp>
 #include <boost/throw_exception.hpp>
-#include <functional>
 #include <ostream>
 #include <stdexcept>
-#include <tuple>
 #include <type_traits>
 #include <utility>
 
@@ -31,54 +29,39 @@ namespace boost {
 namespace histogram {
 namespace detail {
 
-template <class T, class U>
-struct ref_handler_impl {
-  using type = T;
-  template <class V>
-  static V unpack(V&& v) {
-    return std::forward<V>(v);
-  }
-};
-
-template <class T, class U>
-struct ref_handler_impl<T, std::reference_wrapper<U>> {
-  using type = std::reference_wrapper<T>;
-  template <class V>
-  static auto unpack(V&& v) {
-    return v ? &(v->get()) : nullptr;
-  }
-};
-
-template <class T, class U>
-struct ref_handler_impl<T, std::reference_wrapper<const U>> {
-  using type = std::reference_wrapper<const T>;
-  template <class V>
-  static auto unpack(V&& v) {
-    return v ? &(v->get()) : nullptr;
-  }
-};
-
-template <class T, class V>
-using ref_handler = ref_handler_impl<T, mp11::mp_first<V>>;
-
 struct variant_access {
-  template <class T, class Variant>
+  template <class T, class T0, class Variant>
-  static auto get_if(Variant&& v) noexcept {
+  static auto get_if_impl(mp11::mp_list<T, T0>, Variant* v) noexcept {
-    using H = ref_handler<T, remove_cvref_t<Variant>>;
+    return v->impl.template get_if<T>();
-    return H::unpack(v.impl.template get_if<typename H::type>());
+  }
+
+  template <class T, class T0, class Variant>
+  static auto get_if_impl(mp11::mp_list<T, T0*>, Variant* v) noexcept {
+    auto tp = v->impl.template get_if<mp11::mp_if<std::is_const<T0>, const T*, T*>>();
+    return tp ? *tp : nullptr;
   }
 
   template <class T, class Variant>
-  static decltype(auto) get(Variant&& v) {
+  static auto get_if(Variant* v) noexcept {
-    using H = ref_handler<T, remove_cvref_t<Variant>>;
+    using T0 = mp11::mp_first<std::decay_t<Variant>>;
-    return *H::unpack(&v.impl.template get<typename H::type>());
+    return get_if_impl(mp11::mp_list<T, T0>{}, v);
+  }
+
+  template <class T0, class Visitor, class Variant>
+  static decltype(auto) visit_impl(mp11::mp_identity<T0>, Visitor&& vis, Variant&& v) {
+    return v.impl.visit(vis);
+  }
+
+  template <class T0, class Visitor, class Variant>
+  static decltype(auto) visit_impl(mp11::mp_identity<T0*>, Visitor&& vis, Variant&& v) {
+    return v.impl.visit([&vis](auto&& x) -> decltype(auto) { return vis(*x); });
   }
 
   template <class Visitor, class Variant>
-  static decltype(auto) apply(Visitor&& vis, Variant&& v) {
+  static decltype(auto) visit(Visitor&& vis, Variant&& v) {
-    using H = ref_handler<void, remove_cvref_t<Variant>>;
+    using T0 = mp11::mp_first<std::decay_t<Variant>>;
-    return v.impl.apply(
+    return visit_impl(mp11::mp_identity<T0>{}, std::forward<Visitor>(vis),
-        [&vis](auto&& x) -> decltype(auto) { return vis(*H::unpack(&x)); });
+                      std::forward<Variant>(v));
   }
 };
 
@@ -99,7 +82,7 @@ class variant : public iterator_mixin<variant<Ts...>> {
 
   // maybe metadata_type or const metadata_type, if bounded type is const
   using metadata_type = std::remove_reference_t<decltype(
-      traits::metadata(std::declval<detail::unref_t<mp11::mp_first<impl_type>>>()))>;
+      traits::metadata(std::declval<std::remove_pointer_t<mp11::mp_first<variant>>>()))>;
 
 public:
   // cannot import ctors with using directive, it breaks gcc and msvc
@@ -238,7 +221,7 @@ public:
 
   template <class T>
   bool operator==(const T& t) const {
-    const T* tp = detail::variant_access::template get_if<T>(*this);
+    const T* tp = detail::variant_access::template get_if<T>(this);
     return tp && detail::relaxed_equal(*tp, t);
   }
 
@@ -261,49 +244,55 @@ class variant<> {};
 /// Apply visitor to variant (reference).
 template <class Visitor, class... Us>
 decltype(auto) visit(Visitor&& vis, variant<Us...>& var) {
-  return detail::variant_access::apply(vis, var);
+  return detail::variant_access::visit(vis, var);
 }
 
 /// Apply visitor to variant (movable reference).
 template <class Visitor, class... Us>
 decltype(auto) visit(Visitor&& vis, variant<Us...>&& var) {
-  return detail::variant_access::apply(vis, std::move(var));
+  return detail::variant_access::visit(vis, std::move(var));
 }
 
 /// Apply visitor to variant (const reference).
 template <class Visitor, class... Us>
 decltype(auto) visit(Visitor&& vis, const variant<Us...>& var) {
-  return detail::variant_access::apply(vis, var);
+  return detail::variant_access::visit(vis, var);
-}
-
-/// Return reference to T, throws unspecified exception if type does not match.
-template <class T, class... Us>
-decltype(auto) get(variant<Us...>& v) {
-  return detail::variant_access::template get<T>(v);
-}
-
-/// Return movable reference to T, throws unspecified exception if type does not match.
-template <class T, class... Us>
-decltype(auto) get(variant<Us...>&& v) {
-  return std::move(detail::variant_access::template get<T>(v));
-}
-
-/// Return const reference to T, throws unspecified exception if type does not match.
-template <class T, class... Us>
-decltype(auto) get(const variant<Us...>& v) {
-  return detail::variant_access::template get<T>(v);
 }
 
 /// Returns pointer to T in variant or null pointer if type does not match.
 template <class T, class... Us>
 T* get_if(variant<Us...>* v) {
-  return detail::variant_access::template get_if<T>(*v);
+  return detail::variant_access::template get_if<T>(v);
 }
 
 /// Returns pointer to const T in variant or null pointer if type does not match.
 template <class T, class... Us>
 const T* get_if(const variant<Us...>* v) {
-  return detail::variant_access::template get_if<T>(*v);
+  return detail::variant_access::template get_if<T>(v);
+}
+
+/// Return reference to T, throws std::runtime_error if type does not match.
+template <class T, class... Us>
+decltype(auto) get(variant<Us...>& v) {
+  auto tp = get_if<T>(&v);
+  if (!tp) BOOST_THROW_EXCEPTION(std::runtime_error("T is not the held type"));
+  return *tp;
+}
+
+/// Return movable reference to T, throws unspecified exception if type does not match.
+template <class T, class... Us>
+decltype(auto) get(variant<Us...>&& v) {
+  auto tp = get_if<T>(&v);
+  if (!tp) BOOST_THROW_EXCEPTION(std::runtime_error("T is not the held type"));
+  return std::move(*tp);
+}
+
+/// Return const reference to T, throws unspecified exception if type does not match.
+template <class T, class... Us>
+decltype(auto) get(const variant<Us...>& v) {
+  auto tp = get_if<T>(&v);
+  if (!tp) BOOST_THROW_EXCEPTION(std::runtime_error("T is not the held type"));
+  return *tp;
 }
 
 // pass-through version of get for generic programming

include/boost/histogram/detail/axes.hpp

@@ -17,7 +17,6 @@
 #include <boost/mp11/list.hpp>
 #include <boost/mp11/tuple.hpp>
 #include <boost/throw_exception.hpp>
-#include <functional>
 #include <stdexcept>
 #include <tuple>
 #include <type_traits>
@@ -50,17 +49,16 @@ template <class... Ts>
 decltype(auto) axis_get(std::tuple<Ts...>& axes, unsigned i) {
   using namespace boost::mp11;
   constexpr auto S = sizeof...(Ts);
-  using V = mp_unique<axis::variant<std::reference_wrapper<Ts>...>>;
+  using V = mp_unique<axis::variant<Ts*...>>;
-  return mp_with_index<S>(i, [&](auto I) { return V(std::ref(std::get<I>(axes))); });
+  return mp_with_index<S>(i, [&axes](auto i) { return V(&std::get<i>(axes)); });
 }
 
 template <class... Ts>
 decltype(auto) axis_get(const std::tuple<Ts...>& axes, unsigned i) {
   using namespace boost::mp11;
   constexpr auto S = sizeof...(Ts);
-  using L = mp_unique<mp_list<std::reference_wrapper<const Ts>...>>;
+  using V = mp_unique<axis::variant<const Ts*...>>;
-  using V = mp_rename<L, axis::variant>;
+  return mp_with_index<S>(i, [&axes](auto i) { return V(&std::get<i>(axes)); });
-  return mp_with_index<S>(i, [&](auto I) { return V(std::cref(std::get<I>(axes))); });
 }
 
 template <class T>

include/boost/histogram/detail/variant.hpp

@@ -8,7 +8,8 @@
 #ifndef BOOST_HISTOGRAM_DETAIL_VARIANT_HPP
 #define BOOST_HISTOGRAM_DETAIL_VARIANT_HPP
 
-#include <boost/mp11.hpp>
+#include <boost/mp11/algorithm.hpp>
+#include <boost/mp11/list.hpp>
 #include <boost/throw_exception.hpp>
 #include <iosfwd>
 #include <stdexcept>
@@ -25,114 +26,92 @@ T launder_cast(U&& u) {
 
 // Simple C++14 variant that only depends on boost.mp11 and boost.throw_exception.
 //
-// * No empty state, first type should have a default ctor that never throws;
+// * No empty state
-//   if it throws anyway, the program aborts.
+// * All held types must have copy ctors and copy assignment.
-// * All types must have copy ctors and copy assignment.
+// * All held types must have noexcept move ctors and noexcept move assignment.
-// * All types must have noexcept move ctors and noexcept move assignment.
 //
 template <class... Ts>
 class variant {
-  using default_type = mp11::mp_first<variant>;
-  using N = mp11::mp_size<variant>;
-
 public:
-  variant() noexcept { init_default(); }
+  variant() { create<mp11::mp_first<variant>>(0); }
 
-  variant(const variant& x) {
+  variant(variant&& x) noexcept {
-    x.internal_apply([this, &x](auto i) {
+    x.internal_visit([this, &x](auto i) {
       using T = mp11::mp_at_c<variant, i>;
-      this->init_i<T>(i, *x.ptr(mp11::mp_identity<T>{}, i));
+      static_assert(std::is_nothrow_move_constructible<T>::value, "");
+      this->create<T>(i, std::move(x.template ref<T>()));
     });
   }
 
-  variant(variant&& x) noexcept {
+  variant& operator=(variant&& x) noexcept {
-    x.internal_apply([this, &x](auto i) {
+    x.internal_visit([this, &x](auto i) {
       using T = mp11::mp_at_c<variant, i>;
-      this->init_i<T>(i, std::move(*x.ptr(mp11::mp_identity<T>{}, i)));
+      this->operator=(std::move(x.template ref<T>()));
+    });
+    return *this;
+  }
+
+  variant(const variant& x) {
+    x.internal_visit([this, &x](auto i) {
+      using T = mp11::mp_at_c<variant, i>;
+      this->create<T>(i, x.template ref<T>());
     });
   }
 
   variant& operator=(const variant& x) {
-    x.apply([this](const auto& x) { this->operator=(x); });
+    x.visit([this](const auto& x) { this->operator=(x); });
     return *this;
   }
 
-  variant& operator=(variant&& x) noexcept {
+  template <class U, class = std::enable_if_t<(mp11::mp_contains<variant, U>::value &&
-    x.apply([this](auto&& x) { this->operator=(std::move(x)); });
+                                               !std::is_reference<U>::value)>>
-    return *this;
-  }
-
-  template <class U, class = std::enable_if_t<mp11::mp_contains<variant, U>::value>>
   explicit variant(U&& x) noexcept {
-    static_assert(std::is_rvalue_reference<decltype(x)>::value, "");
     constexpr auto i = find<U>();
     using T = mp11::mp_at_c<variant, i>;
     static_assert(std::is_nothrow_move_constructible<T>::value, "");
-    init_i<T>(i, std::move(x));
+    create<T>(i, std::move(x));
+  }
+
+  template <class U, class = std::enable_if_t<(mp11::mp_contains<variant, U>::value &&
+                                               !std::is_reference<U>::value)>>
+  variant& operator=(U&& x) noexcept {
+    constexpr auto i = find<U>();
+    using T = mp11::mp_at_c<variant, i>;
+    if (i == index_) {
+      static_assert(std::is_nothrow_move_assignable<T>::value, "");
+      ref<T>() = std::move(x);
+    } else {
+      destroy();
+      static_assert(std::is_nothrow_move_constructible<T>::value, "");
+      create<T>(i, std::move(x));
+    }
+    return *this;
   }
 
   template <class U, class = std::enable_if_t<mp11::mp_contains<variant, U>::value>>
   explicit variant(const U& x) {
     constexpr auto i = find<U>();
     using T = mp11::mp_at_c<variant, i>;
-    init_i<T>(i, x);
+    create<T>(i, x);
-  }
-
-  template <class U, class = std::enable_if_t<mp11::mp_contains<variant, U>::value>>
-  variant& operator=(U&& x) noexcept {
-    constexpr auto i = find<U>();
-    using T = mp11::mp_at_c<variant, i>;
-    static_assert(std::is_nothrow_move_constructible<T>::value, "");
-    if (i == index_) {
-      *ptr(mp11::mp_identity<T>{}, i) = std::move(x);
-    } else {
-      destroy();
-      init_i<T>(i, std::move(x));
-    }
-    return *this;
   }
 
   template <class U, class = std::enable_if_t<mp11::mp_contains<variant, U>::value>>
   variant& operator=(const U& x) {
-    constexpr auto i = find<U>();
+    auto tp = get_if<U>();
-    using T = mp11::mp_at_c<variant, i>;
+    if (tp) {
-    if (i == index_) {
+      *tp = x;
-      // nothing to do if T::operator= throws
-      *ptr(mp11::mp_identity<T>{}, i) = x;
     } else {
-      destroy(); // now in invalid state
+      // Avoid empty state by first calling copy ctor and use move assignment.
-      init_guard guard(*this, i);
+      // Copy ctor may throw, leaving variant in current state. If copy ctor succeeds,
-      init_i<T>(i, x);
+      // move assignment is noexcept.
+      variant v(x);
+      operator=(std::move(v));
     }
     return *this;
   }
 
   ~variant() { destroy(); }
 
-  template <class U>
-  bool operator==(const U& x) const noexcept {
-    constexpr auto i = find<U>();
-    static_assert(i < N::value, "argument is not a bounded type");
-    using T = mp11::mp_at_c<variant, i>;
-    return (i == index_) && *ptr(mp11::mp_identity<T>{}, i) == x;
-  }
-
-  template <class U>
-  bool operator!=(const U& x) const noexcept {
-    constexpr auto i = find<U>();
-    static_assert(i < N::value, "argument is not a bounded type");
-    using T = mp11::mp_at_c<variant, i>;
-    return (i != index_) || *ptr(mp11::mp_identity<T>{}, i) != x;
-  }
-
-  bool operator==(const variant& x) const noexcept {
-    return x.apply([this](const auto& x) { return this->operator==(x); });
-  }
-
-  bool operator!=(const variant& x) const noexcept {
-    return x.apply([this](const auto& x) { return this->operator!=(x); });
-  }
-
   unsigned index() const noexcept { return index_; }
 
   template <class T>
@@ -152,78 +131,57 @@ public:
   template <class T>
   T* get_if() noexcept {
     constexpr auto i = mp11::mp_find<variant, T>{};
-    return i == index_ ? ptr(mp11::mp_identity<T>{}, i) : nullptr;
+    return i == index_ ? &ref<T>() : nullptr;
   }
 
   template <class T>
   const T* get_if() const noexcept {
     constexpr auto i = mp11::mp_find<variant, T>{};
-    return i == index_ ? ptr(mp11::mp_identity<T>{}, i) : nullptr;
+    return i == index_ ? &ref<T>() : nullptr;
   }
 
   template <class Functor>
-  decltype(auto) apply(Functor&& functor) const {
+  decltype(auto) visit(Functor&& functor) const {
-    return internal_apply([this, &functor](auto i) -> decltype(auto) {
+    return internal_visit([this, &functor](auto i) -> decltype(auto) {
       using T = mp11::mp_at_c<variant, i>;
-      return functor(*(this->ptr(mp11::mp_identity<T>{}, i)));
+      return functor(this->template ref<T>());
     });
   }
 
   template <class Functor>
-  decltype(auto) apply(Functor&& functor) {
+  decltype(auto) visit(Functor&& functor) {
-    return internal_apply([this, &functor](auto i) -> decltype(auto) {
+    return internal_visit([this, &functor](auto i) -> decltype(auto) {
       using T = mp11::mp_at_c<variant, i>;
-      return functor(*(this->ptr(mp11::mp_identity<T>{}, i)));
+      return functor(this->template ref<T>());
     });
   }
 
 private:
-  class init_guard {
-  public:
-    init_guard(variant& v, unsigned i) noexcept
-      : v_(v), i_(i) {}
-
-    ~init_guard() {
-      if (v_.index_ != i_) {
-        v_.init_default();
-      }
-    }
-
-    init_guard(const init_guard&) = delete;
-    init_guard& operator=(const init_guard&) = delete;
-
-  private:
-    variant& v_;
-    unsigned i_;
-  };
-
   template <class Functor>
-  decltype(auto) internal_apply(Functor&& functor) const {
+  decltype(auto) internal_visit(Functor&& functor) const {
     return mp11::mp_with_index<sizeof...(Ts)>(index_, functor);
   }
 
-  template <class T, std::size_t N>
+  template <class T>
-  T* ptr(mp11::mp_identity<T>, mp11::mp_size_t<N>) noexcept {
+  T& ref() noexcept {
-    return launder_cast<T*>(&buffer_);
+    return launder_cast<T&>(buffer_);
   }
 
-  template <class T, std::size_t N>
+  template <class T>
-  const T* ptr(mp11::mp_identity<T>, mp11::mp_size_t<N>) const noexcept {
+  const T& ref() const noexcept {
-    return launder_cast<const T*>(&buffer_);
+    return launder_cast<const T&>(buffer_);
   }
 
-  void init_default() noexcept { init_i<default_type>(mp11::mp_size_t<0>{}); }
+  template <class T, class... Args>
-
+  void create(unsigned i, Args&&... args) {
-  template <class T, class I, class... Args>
-  void init_i(I, Args&&... args) {
     new (&buffer_) T(std::forward<Args>(args)...);
-    index_ = I::value;
+    index_ = i;
   }
 
-  void destroy() noexcept {
+  void destroy() {
-    internal_apply([this](auto i) {
+    internal_visit([this](auto i) {
       using T = mp11::mp_at_c<variant, i>;
-      this->ptr(mp11::mp_identity<T>{}, i)->~T();
+      this->template ref<T>().~T();
     });
   }
 
@@ -241,7 +199,7 @@ private:
 template <class CharT, class Traits, class... Ts>
 std::basic_ostream<CharT, Traits>& operator<<(std::basic_ostream<CharT, Traits>& os,
                                               const variant<Ts...>& x) {
-  x.apply([&os](const auto& self) { os << self; });
+  x.visit([&os](const auto& self) { os << self; });
   return os;
 }
 

test/axis_variant_test.cpp

@@ -13,7 +13,6 @@
 #include <boost/histogram/detail/cat.hpp>
 #include <boost/histogram/detail/throw_exception.hpp>
 #include <boost/histogram/detail/type_name.hpp>
-#include <functional>
 #include <sstream>
 #include <string>
 #include <type_traits>
@@ -24,14 +23,6 @@
 using namespace boost::histogram;
 namespace tr = axis::transform;
 
-namespace std {
-template <class T>
-ostream& operator<<(ostream& os, const reference_wrapper<T>& t) {
-  os << t.get();
-  return os;
-}
-} // namespace std
-
 int main() {
   { (void)axis::variant<>{}; }
 
@@ -62,13 +53,13 @@ int main() {
     BOOST_TEST_EQ(a.options(), axis::option::overflow_t::value);
   }
 
-  // axis::variant with std::reference_wrapper
+  // axis::variant with pointers
   {
     using A = axis::integer<>;
     using B = axis::regular<>;
     auto a = A(1, 5, "foo");
     auto b = B(3, 1, 5, "bar");
-    axis::variant<std::reference_wrapper<A>, std::reference_wrapper<B>> r1(std::ref(a));
+    axis::variant<A*, B*> r1(&a);
     BOOST_TEST_EQ(r1, a);
     BOOST_TEST_NE(r1, A(2, 4));
     BOOST_TEST_NE(r1, b);
@@ -79,11 +70,10 @@ int main() {
     // change original through r1
     axis::get<A>(r1).metadata() = "bar";
     BOOST_TEST_EQ(a.metadata(), "bar");
-    r1 = std::ref(b);
+    r1 = &b;
     BOOST_TEST_EQ(r1, b);
 
-    axis::variant<std::reference_wrapper<const A>, std::reference_wrapper<const B>> r2(
+    axis::variant<const A*, const B*> r2(static_cast<const B*>(&b));
-        std::cref(b));
     BOOST_TEST_EQ(r2, b);
     BOOST_TEST_NE(r2, B(4, 1, 5));
     BOOST_TEST_NE(r2, a);

test/detail_misc_test.cpp

@@ -42,7 +42,7 @@ int main() {
     auto a1 = axis::integer<>(0, 1);
     auto a2 = axis::integer<>(1, 2);
     auto tup = std::make_tuple(a1, a2);
-    using E1 = axis::variant<std::reference_wrapper<axis::integer<>>>;
+    using E1 = axis::variant<axis::integer<>*>;
     BOOST_TEST_TRAIT_SAME(decltype(detail::axis_get(tup, 0)), E1);
     BOOST_TEST_EQ(detail::axis_get(tup, 0), a1);
     BOOST_TEST_EQ(detail::axis_get(tup, 1), a2);

test/detail_variant_test.cpp

@@ -56,6 +56,11 @@ struct Q {
 template <int N>
 int Q<N>::count = 0;
 
+template <class... Ts, class U>
+bool operator==(const variant<Ts...>& v, const U& u) {
+  return v.visit([&u](const auto& x) { return x == u; });
+}
+
 int main() {
   // test Q
   BOOST_TEST_EQ(Q<1>::count, 0);
@@ -176,16 +181,14 @@ int main() {
     v = q2;
     BOOST_TEST_EQ(v.index(), 1);
     BOOST_TEST_EQ(v, q2);
-    BOOST_TEST_NOT(v.get<Q<2>>().moved);
     BOOST_TEST_EQ(Q<1>::count, 1);
     BOOST_TEST_EQ(Q<2>::count, 2);
 
     BOOST_TEST_EQ(v.index(), 1);
-#ifndef BOOST_NO_EXCEPTIONS
     Q<3> q3(0xBAD);
     BOOST_TEST_THROWS(v = q3, std::bad_alloc);
-    BOOST_TEST_EQ(v.index(), 0); // is now in default state
+    BOOST_TEST_EQ(v.index(), 1); // still in previous state
-#endif
+    BOOST_TEST_EQ(v, q2);
   }
   BOOST_TEST_EQ(Q<1>::count, 0);
   BOOST_TEST_EQ(Q<2>::count, 0);
@@ -214,17 +217,17 @@ int main() {
     BOOST_TEST_NOT(crv.get_if<int>());
   }
 
-  // apply
+  // visit
   {
     variant<Q<1>, Q<2>> v;
     v = Q<1>(1);
-    v.apply([](auto& x) {
+    v.visit([](auto& x) {
       BOOST_TEST_EQ(x, 1);
       BOOST_TEST_TRAIT_SAME(decltype(x), Q<1>&);
     });
     v = Q<2>(2);
     const auto& crv = v;
-    crv.apply([](const auto& x) {
+    crv.visit([](const auto& x) {
       BOOST_TEST_EQ(x, 2);
       BOOST_TEST_TRAIT_SAME(decltype(x), const Q<2>&);
     });