Address PR#83 review

* Makes EXPECTS() fail handler a global, settable function pointer.
* Fix `util::upto` for bidirectional iterators with begin==end.
* Use `std::string` in error reporting with `either` in `partition_view`
  to correct incorrect default conversion in return value.
* Correct return types for `util::range::operator[]` and `util::range::at()`
* `transform_iterator` requires only iterator==sentinel test for correct sentinel
  behaviour.
* Extend partition tests to cover:
  * partition views over short (length) sequences,
  * non-numeric partitions,
  * throw on validation check of non-monotonic sequence,
  * make_partition with short and long size sequences, short division containers.
* Extend range tests to cover:
  * compatibility with `std::accumulate`,
  * correct constness of dereferences with respect to wrapped iterators,
  * proper type deduction for `make_range` over pointers.
* Extend span tests to cover:
  * span creation from `std::pair`,
  * proper type promotion with heterogeneous bounds.

src/util/debug.cpp

@@ -14,8 +14,11 @@ namespace util {
 
 std::mutex global_debug_cerr_mutex;
 
-bool failed_assertion(const char* assertion, const char* file,
-                      int line, const char* func)
+bool abort_on_failed_assertion(
+    const char* assertion,
+    const char* file,
+    int line,
+    const char* func)
 {
     // Explicit flush, as we can't assume default buffering semantics on stderr/cerr,
     // and abort() might not flush streams.
@@ -26,8 +29,13 @@ bool failed_assertion(const char* assertion, const char* file,
     return false;
 }
 
-std::ostream& debug_emit_trace_leader(std::ostream& out, const char* file,
-                                      int line, const char* varlist)
+failed_assertion_handler_t global_failed_assertion_handler = abort_on_failed_assertion;
+
+std::ostream& debug_emit_trace_leader(
+    std::ostream& out,
+    const char* file,
+    int line,
+    const char* varlist)
 {
     iosfmt_guard guard(out);
 

src/util/debug.hpp

@@ -10,7 +10,17 @@ namespace nest {
 namespace mc {
 namespace util {
 
-bool failed_assertion(const char* assertion, const char* file, int line, const char* func);
+using failed_assertion_handler_t =
+    bool (*)(const char* assertion, const char* file, int line, const char* func);
+
+bool abort_on_failed_assertion(const char* assertion, const char* file, int line, const char* func);
+inline bool ignore_failed_assertion(const char*, const char*, int, const char*) {
+    return false;
+}
+
+// defaults to abort_on_failed_assertion;
+extern failed_assertion_handler_t global_failed_assertion_handler;
+
 std::ostream& debug_emit_trace_leader(std::ostream& out, const char* file, int line, const char* varlist);
 
 inline void debug_emit(std::ostream& out) {
@@ -66,7 +76,7 @@ void debug_emit_trace(const char* file, int line, const char* varlist, const Arg
 
     #define EXPECTS(condition) \
        (void)((condition) || \
-       nest::mc::util::failed_assertion(#condition, __FILE__, __LINE__, DEBUG_FUNCTION_NAME))
+       nest::mc::util::global_failed_assertion_handler(#condition, __FILE__, __LINE__, DEBUG_FUNCTION_NAME))
 #else
     #define EXPECTS(condition)
 #endif // def WITH_ASSERTIONS

src/util/iterutil.hpp

@@ -40,7 +40,7 @@ upto(I iter, E end) {
 template <typename I, typename E>
 enable_if_t<is_bidirectional_iterator<E>::value && std::is_constructible<I, E>::value, I>
 upto(I iter, E end) {
-    return I{std::prev(end)};
+    return iter==I{end}? iter: I{std::prev(end)};
 }
 
 /*

src/util/partition.hpp

@@ -75,7 +75,7 @@ public:
 private:
     either<bool, std::string> is_valid() const {
         if (!std::is_sorted(left.get(), right.get())) {
-            return "offsets are not monotonically increasing";
+            return std::string("offsets are not monotonically increasing");
         }
         else {
             return true;

src/util/range.hpp

@@ -48,18 +48,20 @@ struct range {
     using difference_type = typename std::iterator_traits<iterator>::difference_type;
     using size_type = typename std::make_unsigned<difference_type>::type;
     using value_type = typename std::iterator_traits<iterator>::value_type;
+    using reference = typename std::iterator_traits<iterator>::reference;
     using const_reference = const value_type&;
-    using reference = const_reference;
 
-    U left;
-    S right;
+    iterator left;
+    sentinel right;
 
     range() = default;
     range(const range&) = default;
     range(range&&) = default;
 
     template <typename U1, typename U2>
-    range(U1&& l, U2&& r): left(std::forward<U1>(l)), right(std::forward<U2>(r)) {}
+    range(U1&& l, U2&& r):
+        left(std::forward<U1>(l)), right(std::forward<U2>(r))
+    {}
 
     range& operator=(const range&) = default;
     range& operator=(range&&) = default;
@@ -78,11 +80,11 @@ struct range {
         return std::distance(begin(), end());
     }
 
-    size_type max_size() const { return std::numeric_limits<size_type>::max(); }
+    constexpr size_type max_size() const { return std::numeric_limits<size_type>::max(); }
 
-    void swap(range<U>& b) {
-        std::swap(left, b.left);
-        std::swap(right, b.right);
+    void swap(range& other) {
+        std::swap(left, other.left);
+        std::swap(right, other.right);
     }
 
     decltype(*left) front() const { return *left; }
@@ -90,13 +92,13 @@ struct range {
     decltype(*left) back() const { return *upto(left, right); }
 
     template <typename V = iterator>
-    enable_if_t<is_random_access_iterator<V>::value, value_type>
+    enable_if_t<is_random_access_iterator<V>::value, decltype(*left)>
     operator[](difference_type n) const {
         return *std::next(begin(), n);
     }
 
     template <typename V = iterator>
-    enable_if_t<is_random_access_iterator<V>::value, value_type>
+    enable_if_t<is_random_access_iterator<V>::value, decltype(*left)>
     at(difference_type n) const {
         if (size_type(n) >= size()) {
             throw std::out_of_range("out of range in range");
@@ -105,16 +107,24 @@ struct range {
     }
 
 #ifdef WITH_TBB
-    template <typename V = iterator,
-              typename = enable_if_t<is_forward_iterator<V>::value>>
-    range(range& r, tbb::split): left(r.left), right(r.right) {
+    template <
+        typename V = iterator,
+        typename = enable_if_t<is_forward_iterator<V>::value>
+    >
+    range(range& r, tbb::split):
+        left(r.left), right(r.right)
+    {
         std::advance(left, r.size()/2u);
         r.right = left;
     }
 
-    template <typename V = iterator,
-              typename = enable_if_t<is_forward_iterator<V>::value>>
-    range(range& r, tbb::proportional_split p): left(r.left), right(r.right) {
+    template <
+        typename V = iterator,
+        typename = enable_if_t<is_forward_iterator<V>::value>
+    >
+    range(range& r, tbb::proportional_split p):
+        left(r.left), right(r.right)
+    {
         size_type i = (r.size()*p.left())/(p.left()+p.right());
         if (i<1) {
             i = 1;
@@ -145,28 +155,28 @@ range<U, V> make_range(const U& left, const V& right) {
  */
 template <typename I, typename S>
 class sentinel_iterator {
-    nest::mc::util::either<I, S> e;
+    nest::mc::util::either<I, S> e_;
 
-    bool is_sentinel() const { return e.index()!=0; }
+    bool is_sentinel() const { return e_.index()!=0; }
 
     I& iter() {
         EXPECTS(!is_sentinel());
-        return e.template unsafe_get<0>();
+        return e_.template unsafe_get<0>();
     }
 
     const I& iter() const {
         EXPECTS(!is_sentinel());
-        return e.template unsafe_get<0>();
+        return e_.template unsafe_get<0>();
     }
 
     S& sentinel() {
         EXPECTS(is_sentinel());
-        return e.template unsafe_get<1>();
+        return e_.template unsafe_get<1>();
     }
 
     const S& sentinel() const {
         EXPECTS(is_sentinel());
-        return e.template unsafe_get<1>();
+        return e_.template unsafe_get<1>();
     }
 
 public:
@@ -176,10 +186,10 @@ public:
     using reference = typename std::iterator_traits<I>::reference;
     using iterator_category = typename std::iterator_traits<I>::iterator_category;
 
-    sentinel_iterator(I i): e(i) {}
+    sentinel_iterator(I i): e_(i) {}
 
     template <typename V = S, typename = enable_if_t<!std::is_same<I, V>::value>>
-    sentinel_iterator(S i): e(i) {}
+    sentinel_iterator(S i): e_(i) {}
 
     sentinel_iterator() = default;
     sentinel_iterator(const sentinel_iterator&) = default;
@@ -192,7 +202,7 @@ public:
 
     auto operator*() const -> decltype(*iter()) { return *iter(); }
 
-    I operator->() const { return e.template ptr<0>(); }
+    I operator->() const { return e_.template ptr<0>(); }
 
     sentinel_iterator& operator++() {
         ++iter();
@@ -298,9 +308,9 @@ sentinel_iterator_t<I, S> make_sentinel_end(const I& i, const S& s) {
 
 template <typename Seq>
 auto canonical_view(const Seq& s) ->
-    range<sentinel_iterator_t<decltype(s.begin()), decltype(s.end())>>
+    range<sentinel_iterator_t<decltype(std::begin(s)), decltype(std::end(s))>>
 {
-    return {make_sentinel_iterator(s.begin(), s.end()), make_sentinel_end(s.begin(), s.end())};
+    return {make_sentinel_iterator(std::begin(s), std::end(s)), make_sentinel_end(std::begin(s), std::end(s))};
 }
 
 /*

src/util/span.hpp

@@ -24,7 +24,7 @@ span<typename std::common_type<I, J>::type> make_span(I left, J right) {
 
 template <typename I, typename J>
 span<typename std::common_type<I, J>::type> make_span(std::pair<I, J> interval) {
-    return span<typename std::common_type<I, J>::type>(interval.left, interval.right);
+    return span<typename std::common_type<I, J>::type>(interval.first, interval.second);
 }
 
 

src/util/transform.hpp

@@ -29,7 +29,7 @@ class transform_iterator: public iterator_adaptor<transform_iterator<I, F>, I> {
     const I& inner() const { return inner_; }
     I& inner() { return inner_; }
 
-    using inner_value_type = decay_t<decltype(*inner_)>;
+    using inner_value_type = util::decay_t<decltype(*inner_)>;
 
 public:
     using typename base::difference_type;
@@ -70,12 +70,12 @@ public:
     bool operator==(const Sentinel& s) const { return inner_==s; }
 
     template <typename Sentinel>
-    bool operator!=(const Sentinel& s) const { return inner_!=s; }
+    bool operator!=(const Sentinel& s) const { return !(inner_==s); }
 };
 
 template <typename I, typename F>
-transform_iterator<I, decay_t<F>> make_transform_iterator(const I& i, const F& f) {
-    return transform_iterator<I, decay_t<F>>(i, f);
+transform_iterator<I, util::decay_t<F>> make_transform_iterator(const I& i, const F& f) {
+    return transform_iterator<I, util::decay_t<F>>(i, f);
 }
 
 template <
@@ -85,7 +85,7 @@ template <
     typename seq_csent = typename sequence_traits<Seq>::const_sentinel,
     typename = enable_if_t<std::is_same<seq_citer, seq_csent>::value>
 >
-range<transform_iterator<seq_citer, decay_t<F>>>
+range<transform_iterator<seq_citer, util::decay_t<F>>>
 transform_view(const Seq& s, const F& f) {
     return {make_transform_iterator(cbegin(s), f), make_transform_iterator(cend(s), f)};
 }
@@ -98,7 +98,7 @@ template <
     typename seq_csent = typename sequence_traits<Seq>::const_sentinel,
     typename = enable_if_t<!std::is_same<seq_citer, seq_csent>::value>
 >
-range<transform_iterator<seq_citer, decay_t<F>>, seq_csent>
+range<transform_iterator<seq_citer, util::decay_t<F>>, seq_csent>
 transform_view(const Seq& s, const F& f) {
     return {make_transform_iterator(cbegin(s), f), cend(s)};
 }

tests/unit/test_partition.cpp

 #include "gtest.h"
 
+#include <array>
 #include <forward_list>
+#include <string>
 #include <vector>
 
+#include <util/debug.hpp>
+#include <util/nop.hpp>
 #include <util/partition.hpp>
 
 using namespace nest::mc;
@@ -30,6 +34,28 @@ TEST(partition, partition_view) {
     EXPECT_EQ(ends_expected, ends);
 }
 
+TEST(partition, short_partition_view) {
+    int two_divs[] = {10, 15};
+    EXPECT_EQ(1u, util::partition_view(two_divs).size());
+
+    int one_div[] = {10};
+    EXPECT_EQ(0u, util::partition_view(one_div).size());
+
+    std::array<int, 0> zero_divs;
+    EXPECT_EQ(0u, util::partition_view(zero_divs).size());
+}
+
+TEST(partition, check_monotonicity) {
+    // override any EXPECTS checks in partition
+    util::global_failed_assertion_handler = util::ignore_failed_assertion;
+
+    int divs_ok[] = {1, 2, 2, 3, 3};
+    EXPECT_NO_THROW(util::partition_view(divs_ok).validate());
+
+    int divs_bad[] = {3, 2, 1};
+    EXPECT_THROW(util::partition_view(divs_bad).validate(), util::invalid_partition);
+}
+
 TEST(partition, partition_view_find) {
     std::vector<double> divs = { 1, 2.5, 3, 5.5 };
     double eps = 0.1;
@@ -48,6 +74,13 @@ TEST(partition, partition_view_find) {
     EXPECT_EQ(divs[2], p.find(divs[1]+eps)->second);
 }
 
+TEST(partition, partition_view_non_numeric) {
+    std::string divs[] = { "a", "dictionary", "of", "sorted", "words" };
+    auto p = util::partition_view(divs);
+
+    EXPECT_EQ("dictionary", p.find("elephant")->first);
+}
+
 TEST(partition, make_partition_in_place) {
     unsigned sizes[] = { 7, 3, 0, 2 };
     unsigned part_store[util::size(sizes)+1];
@@ -67,9 +100,39 @@ TEST(partition, make_partition_in_place) {
     EXPECT_EQ(std::make_pair(1u, 3u), p[1]);
     EXPECT_EQ(std::make_pair(3u, 3u), p[2]);
     EXPECT_EQ(std::make_pair(3u, 3u), p[3]);
+
+    // with longer sizes sequence
+    unsigned long_sizes[] = {1, 2, 3, 4, 5, 6};
+    p = util::make_partition(util::partition_in_place, part_store, long_sizes, 0u);
+    ASSERT_EQ(4u, p.size());
+    EXPECT_EQ(std::make_pair(0u, 1u), p[0]);
+    EXPECT_EQ(std::make_pair(1u, 3u), p[1]);
+    EXPECT_EQ(std::make_pair(3u, 6u), p[2]);
+    EXPECT_EQ(std::make_pair(6u, 10u), p[3]);
+
+    // with empty sizes sequence
+    std::array<unsigned, 0> no_sizes;
+    p = util::make_partition(util::partition_in_place, part_store, no_sizes, 17u);
+    ASSERT_EQ(4u, p.size());
+    EXPECT_EQ(std::make_pair(17u, 17u), p[0]);
+    EXPECT_EQ(std::make_pair(17u, 17u), p[1]);
+    EXPECT_EQ(std::make_pair(17u, 17u), p[2]);
+    EXPECT_EQ(std::make_pair(17u, 17u), p[3]);
+
+    // with short partition containers
+    unsigned part_store_one[1];
+    p = util::make_partition(util::partition_in_place, part_store_one, sizes, 10u);
+    ASSERT_EQ(0u, p.size());
+    ASSERT_TRUE(p.empty());
+
+    std::array<unsigned,0> part_store_zero;
+    p = util::make_partition(util::partition_in_place, part_store_zero, sizes, 10u);
+    ASSERT_EQ(0u, p.size());
+    ASSERT_TRUE(p.empty());
 }
 
 TEST(partition, make_partition) {
+    // (also tests differing types for sizes and divisiosn)
     unsigned sizes[] = { 7, 3, 0, 2 };
     std::forward_list<double> part_store = { 100.3 };
 

tests/unit/test_range.cpp

@@ -37,6 +37,9 @@ TEST(range, list_iterator) {
     }
 
     EXPECT_EQ(check, sum);
+
+    auto sum2 = std::accumulate(s.begin(), s.end(), 0);
+    EXPECT_EQ(check, sum2);
 }
 
 TEST(range, pointer) {
@@ -45,6 +48,11 @@ TEST(range, pointer) {
     int r = 5;
 
     util::range<int *> s(&xs[l], &xs[r]);
+    auto s_deduced = util::make_range(xs+l, xs+r);
+
+    EXPECT_TRUE((std::is_same<decltype(s), decltype(s_deduced)>::value));
+    EXPECT_EQ(s.left, s_deduced.left);
+    EXPECT_EQ(s.right, s_deduced.right);
 
     EXPECT_EQ(3u, s.size());
 
@@ -70,6 +78,16 @@ TEST(range, input_iterator) {
     EXPECT_TRUE(std::equal(s.begin(), s.end(), &nums[0]));
 }
 
+TEST(range, const_iterator) {
+    std::vector<int> xs = { 1, 2, 3, 4, 5 };
+    auto r = util::make_range(xs.begin(), xs.end());
+    EXPECT_TRUE((std::is_same<int&, decltype(r.front())>::value));
+
+    const auto& xs_const = xs;
+    auto r_const = util::make_range(xs_const.begin(), xs_const.end());
+    EXPECT_TRUE((std::is_same<const int&, decltype(r_const.front())>::value));
+}
+
 struct null_terminated_t {
     bool operator==(const char *p) const { return !*p; }
     bool operator!=(const char *p) const { return !!*p; }

tests/unit/test_span.cpp

@@ -5,6 +5,7 @@
 #include <list>
 #include <numeric>
 #include <type_traits>
+#include <utility>
 
 #include <util/span.hpp>
 
@@ -59,3 +60,34 @@ TEST(span, int_iterators) {
     EXPECT_EQ(sum, (a+b-1)*(b-a)/2);
 }
 
+TEST(span, make_span) {
+    auto s_empty = util::make_span(3, 3);
+    EXPECT_TRUE(s_empty.empty());
+
+    {
+        auto s = util::make_span((short)3, (unsigned long long)10);
+        auto first = s.front();
+        auto last = s.back();
+
+        EXPECT_EQ(3u, first);
+        EXPECT_EQ(9u, last);
+
+        EXPECT_TRUE((std::is_same<decltype(first), decltype(last)>::value));
+        EXPECT_TRUE((std::is_same<unsigned long long, decltype(first)>::value));
+    }
+
+    {
+        // type abuse! should promote bool to long in span.
+        std::pair<long, bool> bounds(-3, false);
+        auto s = util::make_span(bounds);
+        auto first = s.front();
+        auto last = s.back();
+
+        EXPECT_EQ(-3, first);
+        EXPECT_EQ(-1, last);
+
+        EXPECT_TRUE((std::is_same<decltype(first), decltype(last)>::value));
+        EXPECT_TRUE((std::is_same<long, decltype(first)>::value));
+    }
+}
+