From f1ac91324601fd28b37bcb45df776a80993da7ee Mon Sep 17 00:00:00 2001
From: Hans Dembinski <hans.dembinski@gmail.com>
Date: Sun, 13 Jan 2019 23:18:39 +0100
Subject: [PATCH] better solution for adding optional update method when axis
 has growth

---
 include/boost/histogram/axis/integer.hpp  | 84 ++++++++++++++---------
 include/boost/histogram/detail/meta.hpp   |  4 +-
 include/boost/histogram/serialization.hpp |  2 +-
 include/boost/histogram/unsafe_access.hpp |  8 +--
 test/histogram_growing_test.cpp           |  2 -
 test/meta_test.cpp                        | 12 +---
 6 files changed, 57 insertions(+), 55 deletions(-)

diff --git a/include/boost/histogram/axis/integer.hpp b/include/boost/histogram/axis/integer.hpp
index 0ca77a7c..940d75c8 100644
--- a/include/boost/histogram/axis/integer.hpp
+++ b/include/boost/histogram/axis/integer.hpp
@@ -22,29 +22,21 @@
 namespace boost {
 namespace histogram {
 namespace axis {
-/** Axis for an interval of integer values with unit steps.
- *
- * Binning is a O(1) operation. This axis operates
- * faster than a regular axis.
- */
-template <typename IntType, typename MetaData, option Options>
-class integer : public base<MetaData, Options>,
-                public iterator_mixin<integer<IntType, MetaData, Options>> {
+
+template <class IntType, class MetaData, option Options>
+class integer_base : public base<MetaData, Options>,
+                     public iterator_mixin<integer<IntType, MetaData, Options>> {
   static_assert(!test(Options, option::circular) || !test(Options, option::underflow),
                 "circular axis cannot have underflow");
   static_assert(!std::is_integral<IntType>::value || std::is_same<IntType, int>::value,
                 "integer axis requires type floating point type or int");
   using base_type = base<MetaData, Options>;
-
-public:
   using metadata_type = MetaData;
   using value_type = IntType;
-
-private:
   using index_type = std::conditional_t<std::is_integral<value_type>::value, int, double>;
 
 public:
-  integer() = default;
+  integer_base() = default;
 
   /** Construct over semi-open integer interval [start, stop).
    *
@@ -53,13 +45,13 @@ public:
    * \param metadata description of the axis.
    * \param options  extra bin options.
    */
-  integer(value_type start, value_type stop, metadata_type m = {})
+  integer_base(value_type start, value_type stop, metadata_type m = {})
       : base_type(static_cast<unsigned>(stop - start > 0 ? stop - start : 0),
                   std::move(m))
       , min_(start) {}
 
   /// Constructor used by algorithm::reduce to shrink and rebin.
-  integer(const integer& src, int begin, int end, unsigned merge)
+  integer_base(const integer_base& src, int begin, int end, unsigned merge)
       : base_type(end - begin, src.metadata()), min_(src.min_ + begin) {
     if (merge > 1)
       BOOST_THROW_EXCEPTION(std::invalid_argument("cannot merge bins for integer axis"));
@@ -72,11 +64,6 @@ public:
     return index_impl(std::is_floating_point<value_type>(), x);
   }
 
-  template <option O = Options, class = std::enable_if_t<test(O, option::growth)>>
-  auto update(value_type x) {
-    return update_impl(std::is_floating_point<value_type>(), x);
-  }
-
   /// Returns axis value for index.
   value_type value(index_type i) const noexcept {
     if (!test(Options, option::circular)) {
@@ -90,11 +77,11 @@ public:
     return subscript_impl(std::is_same<index_type, double>(), idx);
   }
 
-  bool operator==(const integer& o) const noexcept {
+  bool operator==(const integer_base& o) const noexcept {
     return base_type::operator==(o) && min_ == o.min_;
   }
 
-  bool operator!=(const integer& o) const noexcept { return !operator==(o); }
+  bool operator!=(const integer_base& o) const noexcept { return !operator==(o); }
 
   template <class Archive>
   void serialize(Archive&, unsigned);
@@ -122,15 +109,54 @@ protected:
     return base_type::size();
   }
 
+  decltype(auto) subscript_impl(std::true_type, int idx) const noexcept {
+    return interval_view<integer_base>(*this, idx);
+  }
+
+  decltype(auto) subscript_impl(std::false_type, int idx) const noexcept {
+    return value(idx);
+  }
+
+  int min_ = 0;
+};
+
+/** Axis for an interval of integer values with unit steps.
+ *
+ * Binning is a O(1) operation. This axis operates
+ * faster than a regular axis.
+ */
+template <class IntType, class MetaData, option Options>
+class integer : public integer_base<IntType, MetaData, Options> {
+  using base_type = integer_base<IntType, MetaData, Options>;
+
+public:
+  using base_type::base_type;
+};
+
+template <class IntType, class MetaData>
+class integer<IntType, MetaData, option::growth>
+    : public integer_base<IntType, MetaData, option::growth> {
+  using base_type = integer_base<IntType, MetaData, option::growth>;
+  using value_type = IntType;
+
+public:
+  using base_type::base_type;
+
+  /// Returns index and shift (if axis has grown) for the passed argument.
+  auto update(value_type x) {
+    return update_impl(std::is_floating_point<value_type>(), x);
+  }
+
+private:
   auto update_impl(std::false_type, int x) noexcept {
-    const auto i = x - min_;
+    const auto i = x - base_type::min_;
     if (i >= 0) {
       if (i < base_type::size()) return std::make_pair(i, 0);
       const auto n = i - base_type::size() + 1;
       base_type::grow(n);
       return std::make_pair(i, -n);
     }
-    min_ += i;
+    base_type::min_ += i;
     base_type::grow(-i);
     return std::make_pair(0, -i);
   }
@@ -142,16 +168,6 @@ protected:
     BOOST_THROW_EXCEPTION(std::invalid_argument("argument is not finite"));
     return std::make_pair(0, 0);
   }
-
-  decltype(auto) subscript_impl(std::true_type, int idx) const noexcept {
-    return interval_view<integer>(*this, idx);
-  }
-
-  decltype(auto) subscript_impl(std::false_type, int idx) const noexcept {
-    return value(idx);
-  }
-
-  int min_ = 0;
 };
 
 #if __cpp_deduction_guides >= 201606
diff --git a/include/boost/histogram/detail/meta.hpp b/include/boost/histogram/detail/meta.hpp
index 80cb783b..28b1d15b 100644
--- a/include/boost/histogram/detail/meta.hpp
+++ b/include/boost/histogram/detail/meta.hpp
@@ -174,9 +174,7 @@ BOOST_HISTOGRAM_MAKE_SFINAE(has_method_lower, &T::lower);
 
 BOOST_HISTOGRAM_MAKE_SFINAE(has_method_value, &T::value);
 
-BOOST_HISTOGRAM_MAKE_SFINAE(
-    has_method_update,
-    (std::declval<T>().update(std::declval<typename T::value_type&>())));
+BOOST_HISTOGRAM_MAKE_SFINAE(has_method_update, (&T::update));
 
 template <typename T>
 using get_value_method_return_type_impl = decltype(std::declval<T&>().value(0));
diff --git a/include/boost/histogram/serialization.hpp b/include/boost/histogram/serialization.hpp
index d927cc9c..883ae037 100644
--- a/include/boost/histogram/serialization.hpp
+++ b/include/boost/histogram/serialization.hpp
@@ -172,7 +172,7 @@ void regular<T, Tr, M, O>::serialize(Archive& ar, unsigned /* version */) {
 
 template <class T, class M, option O>
 template <class Archive>
-void integer<T, M, O>::serialize(Archive& ar, unsigned /* version */) {
+void integer_base<T, M, O>::serialize(Archive& ar, unsigned /* version */) {
   ar& static_cast<base_type&>(*this);
   ar& min_;
 }
diff --git a/include/boost/histogram/unsafe_access.hpp b/include/boost/histogram/unsafe_access.hpp
index 90beb953..34f4595c 100644
--- a/include/boost/histogram/unsafe_access.hpp
+++ b/include/boost/histogram/unsafe_access.hpp
@@ -13,25 +13,25 @@ namespace histogram {
 /// Unsafe read/write access to classes that potentially break consistency
 struct unsafe_access {
   /// Get axes
-  template <typename T>
+  template <class T>
   static typename T::axes_type& axes(T& t) {
     return t.axes_;
   }
 
   /// Get axes (const version)
-  template <typename T>
+  template <class T>
   static const typename T::axes_type& axes(const T& t) {
     return t.axes_;
   }
 
   /// Get storage
-  template <typename T>
+  template <class T>
   static typename T::storage_type& storage(T& t) {
     return t.storage_;
   }
 
   /// Get storage (const version)
-  template <typename T>
+  template <class T>
   static const typename T::storage_type& storage(const T& t) {
     return t.storage_;
   }
diff --git a/test/histogram_growing_test.cpp b/test/histogram_growing_test.cpp
index 2bedb4ec..bc9305c5 100644
--- a/test/histogram_growing_test.cpp
+++ b/test/histogram_growing_test.cpp
@@ -17,8 +17,6 @@ using namespace boost::histogram;
 using integer = axis::integer<int, axis::null_type, axis::option::growth>;
 
 struct category {
-  using value_type = std::string;
-
   int operator()(const std::string& x) const {
     auto it = set_.find(x);
     if (it != set_.end()) return it->second;
diff --git a/test/meta_test.cpp b/test/meta_test.cpp
index 4e4ab29c..e70f3ca7 100644
--- a/test/meta_test.cpp
+++ b/test/meta_test.cpp
@@ -33,13 +33,6 @@ struct VisitorTestFunctor {
   T operator()(T&&);
 };
 
-template <bool B>
-struct OptionalUpdate {
-  using value_type = int;
-  template <bool BB = B, class = std::enable_if_t<BB>>
-  void update(value_type) {}
-};
-
 int main() {
   // has_method_value*
   {
@@ -88,16 +81,13 @@ int main() {
   {
     struct A {};
     struct B {
-      using value_type = int;
-      void update(value_type) {}
+      void update(int) {}
     };
     using C = axis::integer<int, axis::null_type, axis::option::defaults>;
     using D = axis::integer<int, axis::null_type, axis::option::growth>;
 
     BOOST_TEST_TRAIT_FALSE((has_method_update<A>));
     BOOST_TEST_TRAIT_TRUE((has_method_update<B>));
-    BOOST_TEST_TRAIT_FALSE((has_method_update<OptionalUpdate<false>>));
-    BOOST_TEST_TRAIT_TRUE((has_method_update<OptionalUpdate<true>>));
     BOOST_TEST_TRAIT_FALSE((has_method_update<C>));
     BOOST_TEST_TRAIT_TRUE((has_method_update<D>));
   }