From 99354a1c4b6f56871d50d07421ba1773fce0d928 Mon Sep 17 00:00:00 2001 From: Stephan Bergmann Date: Fri, 6 Dec 2019 16:36:01 +0100 Subject: [PATCH 1/2] Adapt o3tl::span to P1872R0 ..."span should have size_type, not index_type" (), as implemented by libc++ since "[libc++][P1872] span should have size_type, not index_type." All uses of index_type had been added to mitigate the previous std::span change from signed (ptrdiff_t) to unsigned (size_t) index_type, see 6ef8420fdbf8dff16de13147c5ab833bc5e01121 "Adapt o3tl::span to updated C++2a std::span". There is no easy solution to transparently support all three std::span variants currently out there (signed index_type, unsigned index_type, unsigned size_type), without causing compilation failures due to CPPUNIT_ASSERT_EQUAL with arguments of different types, or compiler warnings about mixed signed/unsigned comparisons. So rule out the oldest std::span variant (signed index_type) in configure.ac (so that o3tl::span will use its own hand-rolled code in that case) and simplify the uses of index_type to std::size_t (as had already been mentioned in 6ef8420fdbf8dff16de13147c5ab833bc5e01121). Change-Id: I6ddf424ffb7941da3f69ad66fd29ecd35f09afae Reviewed-on: https://gerrit.libreoffice.org/84652 Tested-by: Jenkins Reviewed-by: Stephan Bergmann (cherry picked from commit 8e6865188242bccb3d8aa857ddc990d72a058d3d) --- config_host/config_global.h.in | 3 +++ configure.ac | 16 ++++++++++++++++ include/o3tl/span.hxx | 14 ++++++++------ o3tl/qa/test-span.cxx | 7 ++++--- sfx2/source/control/dispatch.cxx | 3 ++- 5 files changed, 33 insertions(+), 10 deletions(-) diff --git a/config_host/config_global.h.in b/config_host/config_global.h.in index 2e986fbe24b6..0b44ad05373e 100644 --- a/config_host/config_global.h.in +++ b/config_host/config_global.h.in @@ -24,6 +24,9 @@ Any change in this header will cause a rebuild of almost everything. /* Guaranteed copy elision (C++17), __cpp_guaranteed_copy_elision (C++2a): */ #define HAVE_CPP_GUARANTEED_COPY_ELISION 0 +// Useable C++2a : +#define HAVE_CPP_SPAN 0 + /* GCC bug "move ctor wrongly chosen in return stmt (derived vs. base)": */ #define HAVE_GCC_BUG_87150 0 diff --git a/configure.ac b/configure.ac index 052782189056..704fff55a7ec 100644 --- a/configure.ac +++ b/configure.ac @@ -6760,6 +6760,22 @@ AC_COMPILE_IFELSE([AC_LANG_SOURCE([ CXXFLAGS=$save_CXXFLAGS AC_LANG_POP([C++]) +AC_MSG_CHECKING([whether $CXX supports C++2a with unsigned size_type]) +AC_LANG_PUSH([C++]) +save_CXXFLAGS=$CXXFLAGS +CXXFLAGS="$CXXFLAGS $CXXFLAGS_CXX11" +AC_COMPILE_IFELSE([AC_LANG_SOURCE([ + #include + #include + // Don't check size_type directly, as it was called index_type before P1872R0: + void f(std::span s) { static_assert(std::is_unsigned_v); }; + ])], [ + AC_DEFINE([HAVE_CPP_SPAN],[1]) + AC_MSG_RESULT([yes]) + ], [AC_MSG_RESULT([no])]) +CXXFLAGS=$save_CXXFLAGS +AC_LANG_POP([C++]) + AC_MSG_CHECKING([whether $CXX has GCC bug 87150]) AC_LANG_PUSH([C++]) save_CXXFLAGS=$CXXFLAGS diff --git a/include/o3tl/span.hxx b/include/o3tl/span.hxx index 1618b86df897..b19d2d847ac7 100644 --- a/include/o3tl/span.hxx +++ b/include/o3tl/span.hxx @@ -12,7 +12,9 @@ #include -#if __has_include() +#include + +#if HAVE_CPP_SPAN #include @@ -40,7 +42,7 @@ public: using iterator = pointer; using const_reverse_iterator = std::reverse_iterator; using reverse_iterator = std::reverse_iterator; - using index_type = std::size_t; + using size_type = std::size_t; using difference_type = std::ptrdiff_t; constexpr span() noexcept : data_(nullptr), size_(0) {} @@ -48,7 +50,7 @@ public: template constexpr span (T (&a)[N]) noexcept : data_(a), size_(N) {} - constexpr span (T *a, index_type len) noexcept + constexpr span (T *a, size_type len) noexcept : data_(a), size_(len) { // not terribly sure about this, might need to strengthen it @@ -72,9 +74,9 @@ public: { return rbegin(); } constexpr const_reverse_iterator crend() const noexcept { return rend(); } - constexpr index_type size() const noexcept { return size_; } + constexpr size_type size() const noexcept { return size_; } - constexpr reference operator [](index_type pos) const { + constexpr reference operator [](size_type pos) const { assert(pos < size()); return data_[pos]; } @@ -83,7 +85,7 @@ public: private: pointer data_; - index_type size_; + size_type size_; }; } // namespace o3tl diff --git a/o3tl/qa/test-span.cxx b/o3tl/qa/test-span.cxx index 7ec67fa7fd91..3cb78ace1db2 100644 --- a/o3tl/qa/test-span.cxx +++ b/o3tl/qa/test-span.cxx @@ -9,6 +9,7 @@ #include +#include #include #include @@ -42,7 +43,7 @@ private: CPPUNIT_ASSERT_EQUAL(3, *v.crbegin()); CPPUNIT_ASSERT_EQUAL( o3tl::span::difference_type(3), v.crend() - v.crbegin()); - CPPUNIT_ASSERT_EQUAL(o3tl::span::index_type(3), v.size()); + CPPUNIT_ASSERT_EQUAL(std::size_t(3), v.size()); CPPUNIT_ASSERT(!v.empty()); CPPUNIT_ASSERT_EQUAL(2, v[1]); CPPUNIT_ASSERT_EQUAL(1, *v.data()); @@ -52,8 +53,8 @@ private: o3tl::span v1( d1 ); o3tl::span v2( d2 ); std::swap(v1, v2); - CPPUNIT_ASSERT_EQUAL(o3tl::span::index_type(4), v1.size()); - CPPUNIT_ASSERT_EQUAL(o3tl::span::index_type(2), v2.size()); + CPPUNIT_ASSERT_EQUAL(std::size_t(4), v1.size()); + CPPUNIT_ASSERT_EQUAL(std::size_t(2), v2.size()); } } }; diff --git a/sfx2/source/control/dispatch.cxx b/sfx2/source/control/dispatch.cxx index 702e6064f157..c3f3ad60c10c 100644 --- a/sfx2/source/control/dispatch.cxx +++ b/sfx2/source/control/dispatch.cxx @@ -20,6 +20,7 @@ #include #include +#include #include #include @@ -1547,7 +1548,7 @@ void SfxDispatcher::SetSlotFilter(SfxSlotFilterState nEnable, { #ifdef DBG_UTIL // Check Array - for ( o3tl::span::index_type n = 1; n < pSIDs.size(); ++n ) + for ( std::size_t n = 1; n < pSIDs.size(); ++n ) DBG_ASSERT( pSIDs[n] > pSIDs[n-1], "SetSlotFilter: SIDs not sorted" ); #endif From 3bf38cc5f6d61f43509d8dc500479d66b8233bab Mon Sep 17 00:00:00 2001 From: Stephan Bergmann Date: Sat, 22 Feb 2020 17:29:15 +0100 Subject: [PATCH 2/2] Adapt o3tl::span to removal of std::span::cbegin et al "span::cbegin/cend methods produce different results than std::[ranges::]cbegin/cend", as implemented now in "libstdc++: Remove std::span::cbegin and std::span::cend (LWG 3320)". Turns out we only used the removed member functions in o3tl/qa/test-span.cxx. Change-Id: I6c73797594b4e0e753a88840033d54961e271df5 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/89261 Tested-by: Jenkins Reviewed-by: Stephan Bergmann (cherry picked from commit 6fbfad6b00e8c35346ee59cd32a0d7ccc0d8c19c) --- include/o3tl/span.hxx | 7 ------- o3tl/qa/test-span.cxx | 6 ------ 2 files changed, 13 deletions(-) diff --git a/include/o3tl/span.hxx b/include/o3tl/span.hxx index b19d2d847ac7..8af8ba846b65 100644 --- a/include/o3tl/span.hxx +++ b/include/o3tl/span.hxx @@ -62,18 +62,11 @@ public: constexpr iterator begin() const noexcept { return data_; } constexpr iterator end() const noexcept { return begin() + size(); } - constexpr const_iterator cbegin() const noexcept { return begin(); } - constexpr const_iterator cend() const noexcept { return end(); } - reverse_iterator rbegin() const noexcept { return reverse_iterator(end()); } reverse_iterator rend() const noexcept { return reverse_iterator(begin()); } - constexpr const_reverse_iterator crbegin() const noexcept - { return rbegin(); } - constexpr const_reverse_iterator crend() const noexcept { return rend(); } - constexpr size_type size() const noexcept { return size_; } constexpr reference operator [](size_type pos) const { diff --git a/o3tl/qa/test-span.cxx b/o3tl/qa/test-span.cxx index 3cb78ace1db2..26eedfc21938 100644 --- a/o3tl/qa/test-span.cxx +++ b/o3tl/qa/test-span.cxx @@ -34,15 +34,9 @@ private: CPPUNIT_ASSERT_EQUAL(1, *v.begin()); CPPUNIT_ASSERT_EQUAL( o3tl::span::difference_type(3), v.end() - v.begin()); - CPPUNIT_ASSERT_EQUAL(1, *v.cbegin()); - CPPUNIT_ASSERT_EQUAL( - o3tl::span::difference_type(3), v.cend() - v.cbegin()); CPPUNIT_ASSERT_EQUAL(3, *v.rbegin()); CPPUNIT_ASSERT_EQUAL( o3tl::span::difference_type(3), v.rend() - v.rbegin()); - CPPUNIT_ASSERT_EQUAL(3, *v.crbegin()); - CPPUNIT_ASSERT_EQUAL( - o3tl::span::difference_type(3), v.crend() - v.crbegin()); CPPUNIT_ASSERT_EQUAL(std::size_t(3), v.size()); CPPUNIT_ASSERT(!v.empty()); CPPUNIT_ASSERT_EQUAL(2, v[1]);