From a50de751b4a6c94e5ab78b0f99a1d810b8b12e61 Mon Sep 17 00:00:00 2001
From: Sam Yates <halfflat@gmail.com>
Date: Tue, 4 Oct 2016 18:24:17 +0200
Subject: [PATCH] Can't std::sort on forward iterators.
But GNU libstc++ tries to anyway, hilarious segfaults
ensue.
* Don't try and sort a sentinel-terminated range.
* Instead provide a `strict_view` that walks a range
(if required) and presents a range with a proper
end iterator.
* Tests, const range (but non-const iterator) implementations.
---
src/util/range.hpp | 18 ++++++++++++++++++
src/util/rangeutil.hpp | 26 ++++++++++++++++++++++++--
tests/unit/CMakeLists.txt | 12 +-----------
tests/unit/test_range.cpp | 17 +++++++++++++++--
4 files changed, 58 insertions(+), 15 deletions(-)
diff --git a/src/util/range.hpp b/src/util/range.hpp
index 2ed703df..6c161fda 100644
--- a/src/util/range.hpp
+++ b/src/util/range.hpp
@@ -151,6 +151,9 @@ range<U, V> make_range(const U& left, const V& right) {
return range<U, V>(left, right);
}
+// Present a possibly sentinel-terminated range as an STL-compatible sequence
+// using the sentinel_iterator adaptor.
+
template <typename Seq>
auto canonical_view(Seq& s) ->
range<sentinel_iterator_t<decltype(std::begin(s)), decltype(std::end(s))>>
@@ -165,6 +168,21 @@ auto canonical_view(const Seq& s) ->
return {make_sentinel_iterator(std::begin(s), std::end(s)), make_sentinel_end(std::begin(s), std::end(s))};
}
+// Strictly evaluate end point in sentinel-terminated range and present as a range over
+// iterators. Note: O(N) behaviour with forward iterator ranges or sentinel-terminated ranges.
+
+template <typename Seq>
+auto strict_view(Seq& s) -> range<decltype(std::begin(s))>
+{
+ return make_range(std::begin(s), std::next(util::upto(std::begin(s), std::end(s))));
+}
+
+template <typename Seq>
+auto strict_view(const Seq& s) -> range<decltype(std::begin(s))>
+{
+ return make_range(std::begin(s), std::next(util::upto(std::begin(s), std::end(s))));
+}
+
} // namespace util
} // namespace mc
} // namespace nest
diff --git a/src/util/rangeutil.hpp b/src/util/rangeutil.hpp
index 95200b9c..d87edc93 100644
--- a/src/util/rangeutil.hpp
+++ b/src/util/rangeutil.hpp
@@ -75,9 +75,18 @@ AssignableContainer& assign_by(AssignableContainer& c, const Seq& seq, const Pro
}
// Sort in-place
+// Note that a const range reference may wrap non-const iterators.
template <typename Seq>
-void sort(Seq& seq) {
+enable_if_t<!std::is_const<typename util::sequence_traits<Seq>::reference>::value>
+sort(Seq& seq) {
+ auto canon = canonical_view(seq);
+ std::sort(std::begin(canon), std::end(canon));
+}
+
+template <typename Seq>
+enable_if_t<!std::is_const<typename util::sequence_traits<Seq>::reference>::value>
+sort(const Seq& seq) {
auto canon = canonical_view(seq);
std::sort(std::begin(canon), std::end(canon));
}
@@ -85,7 +94,20 @@ void sort(Seq& seq) {
// Sort in-place by projection `proj`
template <typename Seq, typename Proj>
-void sort_by(Seq& seq, const Proj& proj) {
+enable_if_t<!std::is_const<typename util::sequence_traits<Seq>::reference>::value>
+sort_by(Seq& seq, const Proj& proj) {
+ using value_type = typename sequence_traits<Seq>::value_type;
+ auto canon = canonical_view(seq);
+
+ std::sort(std::begin(canon), std::end(canon),
+ [&proj](const value_type& a, const value_type& b) {
+ return proj(a) < proj(b);
+ });
+}
+
+template <typename Seq, typename Proj>
+enable_if_t<!std::is_const<typename util::sequence_traits<Seq>::reference>::value>
+sort_by(const Seq& seq, const Proj& proj) {
using value_type = typename sequence_traits<Seq>::value_type;
auto canon = canonical_view(seq);
diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt
index 1c286863..b67683ec 100644
--- a/tests/unit/CMakeLists.txt
+++ b/tests/unit/CMakeLists.txt
@@ -1,13 +1,3 @@
-set(HEADERS
- ${PROJECT_SOURCE_DIR}/src/cell.hpp
- ${PROJECT_SOURCE_DIR}/src/cell_tree.hpp
- ${PROJECT_SOURCE_DIR}/src/math.hpp
- ${PROJECT_SOURCE_DIR}/src/point.hpp
- ${PROJECT_SOURCE_DIR}/src/segment.hpp
- ${PROJECT_SOURCE_DIR}/src/swcio.hpp
- ${PROJECT_SOURCE_DIR}/src/tree.hpp
-)
-
set(TEST_SOURCES
# unit tests
test_algorithms.cpp
@@ -47,7 +37,7 @@ set(TEST_SOURCES
)
add_definitions("-DDATADIR=\"${CMAKE_SOURCE_DIR}/data\"")
-add_executable(test.exe ${TEST_SOURCES} ${HEADERS})
+add_executable(test.exe ${TEST_SOURCES})
set(TARGETS test.exe)
diff --git a/tests/unit/test_range.cpp b/tests/unit/test_range.cpp
index 3cf2893e..1f1e3cb6 100644
--- a/tests/unit/test_range.cpp
+++ b/tests/unit/test_range.cpp
@@ -184,6 +184,16 @@ TEST(range, sentinel) {
EXPECT_EQ(0u, empty_cstr_range.size());
}
+TEST(range, strictify) {
+ const char *cstr = "hello world";
+ auto cstr_range = util::make_range(cstr, null_terminated);
+
+ auto ptr_range = util::strict_view(cstr_range);
+ EXPECT_TRUE((std::is_same<decltype(ptr_range), util::range<const char *>>::value));
+ EXPECT_EQ(cstr, ptr_range.left);
+ EXPECT_EQ(cstr+11, ptr_range.right);
+}
+
template <typename V>
class counter_range: public ::testing::Test {};
@@ -290,12 +300,15 @@ TEST(range, sort) {
auto cstr_range = util::make_range(std::begin(cstr), null_terminated);
+ // Alas, no forward_iterator sort yet, so make a strict (non-sentinel)
+ // range to sort on below
+
// simple sort
- util::sort(cstr_range);
+ util::sort(util::strict_view(cstr_range));
EXPECT_EQ(std::string("dhowy"), cstr);
// reverse sort by transform c to -c
- util::sort_by(cstr_range, [](char c) { return -c; });
+ util::sort_by(util::strict_view(cstr_range), [](char c) { return -c; });
EXPECT_EQ(std::string("ywohd"), cstr);
}
--
GitLab