From 25608fe86a0f4d8f73a17576749f9779524e755e Mon Sep 17 00:00:00 2001 From: Hans Dembinski Date: Mon, 3 Oct 2022 13:57:08 +0200 Subject: [PATCH] fix sub_array and span in C++20 (#368) --- .github/workflows/cov.yml | 4 + .github/workflows/fast.yml | 4 + .github/workflows/slow.yml | 4 + .github/workflows/superproject_cmake.yml | 4 + benchmark/CMakeLists.txt | 6 +- benchmark/histogram_parallel_filling.cpp | 4 +- include/boost/histogram/detail/span.hpp | 27 ------ include/boost/histogram/detail/sub_array.hpp | 24 ++++-- test/CMakeLists.txt | 1 + test/Jamfile | 1 + test/detail_misc_test.cpp | 16 ---- test/detail_sub_array_and_span_test.cpp | 87 ++++++++++++++++++++ 12 files changed, 127 insertions(+), 55 deletions(-) create mode 100644 test/detail_sub_array_and_span_test.cpp diff --git a/.github/workflows/cov.yml b/.github/workflows/cov.yml index affc33428..438396f05 100644 --- a/.github/workflows/cov.yml +++ b/.github/workflows/cov.yml @@ -14,6 +14,10 @@ env: B2_OPTS: -q -j2 warnings-as-errors=on GCC_VERSION: 11 +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref }} + cancel-in-progress: true + jobs: cov: runs-on: macos-11 diff --git a/.github/workflows/fast.yml b/.github/workflows/fast.yml index 14b514f08..b4219dd66 100644 --- a/.github/workflows/fast.yml +++ b/.github/workflows/fast.yml @@ -20,6 +20,10 @@ on: - 'tools/**' - '*.md' +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref }} + cancel-in-progress: true + jobs: cmake: runs-on: ${{ matrix.os }} diff --git a/.github/workflows/slow.yml b/.github/workflows/slow.yml index 41b03e4f7..d2fc0493e 100644 --- a/.github/workflows/slow.yml +++ b/.github/workflows/slow.yml @@ -10,6 +10,10 @@ on: - 'tools/**' - '*.md' +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref }} + cancel-in-progress: true + env: B2_OPTS: -q -j2 warnings-as-errors=on UBSAN_OPTIONS: print_stacktrace=1 diff --git a/.github/workflows/superproject_cmake.yml b/.github/workflows/superproject_cmake.yml index 9aa8aba59..f97ffb166 100644 --- a/.github/workflows/superproject_cmake.yml +++ b/.github/workflows/superproject_cmake.yml @@ -6,6 +6,10 @@ on: - master - develop +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref }} + cancel-in-progress: true + jobs: build: runs-on: macos-latest diff --git a/benchmark/CMakeLists.txt b/benchmark/CMakeLists.txt index 55bfcb7f6..656e6f08d 100644 --- a/benchmark/CMakeLists.txt +++ b/benchmark/CMakeLists.txt @@ -28,8 +28,10 @@ function(add_benchmark NAME) add_executable(${NAME} ${SOURCE}) target_include_directories(${NAME} PRIVATE ${__INCLUDE_DIRECTORIES}) target_link_libraries(${NAME} PRIVATE Boost::histogram Boost::math benchmark_main ${__LINK_LIBRARIES}) - target_compile_options(${NAME} PRIVATE -DNDEBUG -O3 -march=native -funsafe-math-optimizations ${__COMPILE_OPTIONS}) - + target_compile_options(${NAME} PRIVATE -DNDEBUG -O3 -funsafe-math-optimizations ${__COMPILE_OPTIONS}) + if (NOT DARWIN) + target_compile_options(${NAME} PRIVATE -march=native) + endif() endfunction() add_benchmark(axis_size) diff --git a/benchmark/histogram_parallel_filling.cpp b/benchmark/histogram_parallel_filling.cpp index dfce607e5..9bb002b95 100644 --- a/benchmark/histogram_parallel_filling.cpp +++ b/benchmark/histogram_parallel_filling.cpp @@ -50,12 +50,12 @@ static auto hist = make_histogram_with(DSTS(), axis::regular<>()); static void AtomicStorage(benchmark::State& state) { init.lock(); - if (state.thread_index == 0) { + if (state.thread_index() == 0) { const unsigned nbins = state.range(0); hist = make_histogram_with(DSTS(), axis::regular<>(nbins, 0, 1)); } init.unlock(); - std::default_random_engine gen(state.thread_index); + std::default_random_engine gen(state.thread_index()); std::uniform_real_distribution<> dis(0, 1); for (auto _ : state) { // simulate some work diff --git a/include/boost/histogram/detail/span.hpp b/include/boost/histogram/detail/span.hpp index 663b45756..f5bdb4409 100644 --- a/include/boost/histogram/detail/span.hpp +++ b/include/boost/histogram/detail/span.hpp @@ -7,31 +7,6 @@ #ifndef BOOST_HISTOGRAM_DETAIL_SPAN_HPP #define BOOST_HISTOGRAM_DETAIL_SPAN_HPP -#ifdef __has_include -#if __has_include() -#include -#ifdef __cpp_lib_span -#if __cpp_lib_span >= 201902 -#define BOOST_HISTOGRAM_DETAIL_HAS_STD_SPAN -#endif -#endif -#endif -#endif - -#ifdef BOOST_HISTOGRAM_DETAIL_HAS_STD_SPAN - -#include - -namespace boost { -namespace histogram { -namespace detail { -using std::span; -} // namespace detail -} // namespace histogram -} // namespace boost - -#else // C++17 span not available, so we use our implementation - // to be replaced by boost::span #include @@ -238,8 +213,6 @@ class span : public span_base { } // namespace histogram } // namespace boost -#endif - #include #include diff --git a/include/boost/histogram/detail/sub_array.hpp b/include/boost/histogram/detail/sub_array.hpp index 4bf7ee47d..d60af2b96 100644 --- a/include/boost/histogram/detail/sub_array.hpp +++ b/include/boost/histogram/detail/sub_array.hpp @@ -15,15 +15,17 @@ namespace boost { namespace histogram { namespace detail { +// Like std::array, but allows to use less than maximum capacity. +// Cannot inherit from std::array, since this confuses span. template class sub_array { - constexpr bool swap_element_is_noexcept() noexcept { + static constexpr bool swap_element_is_noexcept() noexcept { using std::swap; return noexcept(swap(std::declval(), std::declval())); } public: - using value_type = T; + using element_type = T; using size_type = std::size_t; using reference = T&; using const_reference = const T&; @@ -42,6 +44,12 @@ class sub_array { fill(value); } + sub_array(std::initializer_list il) noexcept( + std::is_nothrow_assignable::value) + : sub_array(il.size()) { + std::copy(il.begin(), il.end(), data_); + } + reference at(size_type pos) noexcept { if (pos >= size()) BOOST_THROW_EXCEPTION(std::out_of_range{"pos is out of range"}); return data_[pos]; @@ -70,10 +78,7 @@ class sub_array { iterator end() noexcept { return begin() + size_; } const_iterator end() const noexcept { return begin() + size_; } - const_iterator cbegin() noexcept { return data_; } const_iterator cbegin() const noexcept { return data_; } - - const_iterator cend() noexcept { return cbegin() + size_; } const_iterator cend() const noexcept { return cbegin() + size_; } constexpr size_type max_size() const noexcept { return N; } @@ -87,17 +92,20 @@ class sub_array { void swap(sub_array& other) noexcept(swap_element_is_noexcept()) { using std::swap; - for (auto i = begin(), j = other.begin(); i != end(); ++i, ++j) swap(*i, *j); + const size_type s = (std::max)(size(), other.size()); + for (auto i = begin(), j = other.begin(), end = begin() + s; i != end; ++i, ++j) + swap(*i, *j); + swap(size_, other.size_); } private: size_type size_ = 0; - value_type data_[N]; + element_type data_[N]; }; template bool operator==(const sub_array& a, const sub_array& b) noexcept { - return std::equal(a.begin(), a.end(), b.begin()); + return std::equal(a.begin(), a.end(), b.begin(), b.end()); } template diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 371f22820..337da6981 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -73,6 +73,7 @@ boost_test(TYPE run SOURCES detail_operators_test.cpp) boost_test(TYPE run SOURCES detail_relaxed_equal_test.cpp) boost_test(TYPE run SOURCES detail_replace_type_test.cpp) boost_test(TYPE run SOURCES detail_safe_comparison_test.cpp) +boost_test(TYPE run SOURCES detail_sub_array_and_span_test.cpp) boost_test(TYPE run SOURCES detail_static_if_test.cpp) boost_test(TYPE run SOURCES detail_tuple_slice_test.cpp) boost_test(TYPE run SOURCES histogram_custom_axis_test.cpp) diff --git a/test/Jamfile b/test/Jamfile index d3a6b4fc9..d38b5398b 100644 --- a/test/Jamfile +++ b/test/Jamfile @@ -77,6 +77,7 @@ alias cxx14 : [ run detail_replace_type_test.cpp ] [ run detail_safe_comparison_test.cpp ] [ run detail_static_if_test.cpp ] + [ run detail_sub_array_and_span_test.cpp ] [ run detail_tuple_slice_test.cpp ] [ run histogram_custom_axis_test.cpp ] [ run histogram_dynamic_test.cpp ] diff --git a/test/detail_misc_test.cpp b/test/detail_misc_test.cpp index f9287dcf5..ae1dfefce 100644 --- a/test/detail_misc_test.cpp +++ b/test/detail_misc_test.cpp @@ -126,22 +126,6 @@ int main() { BOOST_TEST_EQ(count, 6); } - // sub_array and span - { - dtl::sub_array a(2, 1); - a[1] = 2; - auto sp = dtl::span(a); - BOOST_TEST_EQ(sp.size(), 2); - BOOST_TEST_EQ(sp.front(), 1); - BOOST_TEST_EQ(sp.back(), 2); - - const auto& ca = a; - auto csp = dtl::span(ca); - BOOST_TEST_EQ(csp.size(), 2); - BOOST_TEST_EQ(csp.front(), 1); - BOOST_TEST_EQ(csp.back(), 2); - } - // index_translator { using I = axis::integer<>; diff --git a/test/detail_sub_array_and_span_test.cpp b/test/detail_sub_array_and_span_test.cpp new file mode 100644 index 000000000..00fb916fa --- /dev/null +++ b/test/detail_sub_array_and_span_test.cpp @@ -0,0 +1,87 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include "throw_exception.hpp" + +namespace boost { +namespace histogram { +namespace detail { +template +std::ostream& operator<<(std::ostream& os, const sub_array&) { + return os; +} +std::ostream& operator<<(std::ostream& os, const reduce_command&) { return os; } +} // namespace detail +} // namespace histogram +} // namespace boost + +using namespace boost::histogram::detail; + +int main() { + + { + sub_array a = {1, 2}; + + BOOST_TEST_EQ(a.size(), 2); + BOOST_TEST_EQ(a.max_size(), 3); + BOOST_TEST_EQ(a.at(0), 1); + BOOST_TEST_EQ(a.at(1), 2); + + sub_array b = {1, 2}; + BOOST_TEST_EQ(a, b); + + sub_array c = {1, 2, 3}; + BOOST_TEST_NE(a, c); + + sub_array d = {2, 2}; + BOOST_TEST_NE(a, d); + + auto sp = span(a); + BOOST_TEST_EQ(sp.size(), 2); + BOOST_TEST_EQ(sp.front(), 1); + BOOST_TEST_EQ(sp.back(), 2); + + const auto& ca = a; + auto csp = span(ca); + BOOST_TEST_EQ(csp.size(), 2); + BOOST_TEST_EQ(csp.front(), 1); + BOOST_TEST_EQ(csp.back(), 2); + } + + { + sub_array a(2, 1); + sub_array b(1, 2); + std::swap(a, b); + + BOOST_TEST_EQ(a.size(), 1); + BOOST_TEST_EQ(b.size(), 2); + BOOST_TEST_EQ(a[0], 2); + BOOST_TEST_EQ(b[0], 1); + BOOST_TEST_EQ(b[1], 1); + } + + { + const std::initializer_list a = {1, 2}; + auto sp = span(a); + BOOST_TEST_EQ(sp.size(), 2); + } + + { + const sub_array a(2); + auto sp = span(a); + BOOST_TEST_EQ(sp.size(), 2); + } + + { + const std::initializer_list a = {reduce_command{}}; + auto sp = span(a); + BOOST_TEST_EQ(sp.size(), 1); + } + + return boost::report_errors(); +} \ No newline at end of file