From: Louis Dionne Date: Fri, 27 Aug 2021 16:04:58 +0000 (-0400) Subject: [libc++] Add an assertion in the subrange constructors with a size hint X-Git-Tag: upstream/15.0.7~32268 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=fd66b44ec19e76c111c8a37a42dcb4b44a96bfd6;p=platform%2Fupstream%2Fllvm.git [libc++] Add an assertion in the subrange constructors with a size hint Those constructors are very easy to misuse -- one could easily think that the size passed to the constructor is the size of the range to exhibit from the subrange. Instead, it's a size hint and it's UB to get it wrong. Hence, when it's cheap to compute the real size of the range, it's cheap to make sure that the user didn't get it wrong. Differential Revision: https://reviews.llvm.org/D108827 --- diff --git a/libcxx/include/__ranges/subrange.h b/libcxx/include/__ranges/subrange.h index e162d6b..f42fdd5 100644 --- a/libcxx/include/__ranges/subrange.h +++ b/libcxx/include/__ranges/subrange.h @@ -15,10 +15,11 @@ #include <__concepts/derived_from.h> #include <__concepts/different_from.h> #include <__config> +#include <__debug> +#include <__iterator/advance.h> #include <__iterator/concepts.h> #include <__iterator/incrementable_traits.h> #include <__iterator/iterator_traits.h> -#include <__iterator/advance.h> #include <__ranges/access.h> #include <__ranges/concepts.h> #include <__ranges/dangling.h> @@ -118,7 +119,12 @@ namespace ranges { constexpr subrange(__convertible_to_non_slicing<_Iter> auto __iter, _Sent __sent, make_unsigned_t> __n) requires (_Kind == subrange_kind::sized) - : _Base(_VSTD::move(__iter), __sent, __n) { } + : _Base(_VSTD::move(__iter), __sent, __n) + { + if constexpr (sized_sentinel_for<_Sent, _Iter>) + _LIBCPP_ASSERT((this->__end_ - this->__begin_) == static_cast>(__n), + "std::ranges::subrange was passed an invalid size hint"); + } template<__different_from _Range> requires borrowed_range<_Range> && diff --git a/libcxx/test/std/ranges/range.utility/range.subrange/primitives.pass.cpp b/libcxx/test/std/ranges/range.utility/range.subrange/primitives.pass.cpp index 5cd6d95..b2f135e 100644 --- a/libcxx/test/std/ranges/range.utility/range.subrange/primitives.pass.cpp +++ b/libcxx/test/std/ranges/range.utility/range.subrange/primitives.pass.cpp @@ -15,33 +15,45 @@ #include #include -#include "test_macros.h" #include "test_iterators.h" #include "types.h" constexpr bool test() { - std::ranges::subrange a(MoveOnlyForwardIter(globalBuff), globalBuff + 8, 8); - assert(a.begin().base == globalBuff); - assert(!a.empty()); - assert(a.size() == 8); + int buff[] = {1, 2, 3, 4, 5}; - std::ranges::subrange b(ForwardIter(nullptr), ForwardIter(nullptr)); - assert(b.empty()); + { + std::ranges::subrange a(MoveOnlyForwardIter(buff), buff + 5, 5); + assert(a.begin().base == buff); + assert(!a.empty()); + assert(a.size() == 5); + } - std::ranges::subrange c{ForwardIter(globalBuff), ForwardIter(globalBuff)}; - assert(c.empty()); + { + std::ranges::subrange b(ForwardIter(nullptr), ForwardIter(nullptr)); + assert(b.empty()); + } - std::ranges::subrange d(ForwardIter(globalBuff), ForwardIter(globalBuff + 1)); - assert(!d.empty()); + { + std::ranges::subrange c{ForwardIter(buff), ForwardIter(buff)}; + assert(c.empty()); + } - std::ranges::subrange e(SizedSentinelForwardIter(globalBuff), - SizedSentinelForwardIter(globalBuff + 8), 8); - assert(!e.empty()); - assert(e.size() == 8); + { + std::ranges::subrange d(ForwardIter(buff), ForwardIter(buff + 1)); + assert(!d.empty()); + } - // Make sure that operator- is used to calculate size when possible. - if (!std::is_constant_evaluated()) - assert(SizedSentinelForwardIter::minusCount == 1); + { + bool minusWasCalled = false; + SizedSentinelForwardIter beg(buff, &minusWasCalled), end(buff + 5, &minusWasCalled); + std::ranges::subrange e(beg, end, 5); + assert(!e.empty()); + + // Make sure that operator- is used to calculate size when possible. + minusWasCalled = false; + assert(e.size() == 5); + assert(minusWasCalled); + } return true; } diff --git a/libcxx/test/std/ranges/range.utility/range.subrange/types.h b/libcxx/test/std/ranges/range.utility/range.subrange/types.h index 8dfae99..c07bfff 100644 --- a/libcxx/test/std/ranges/range.utility/range.subrange/types.h +++ b/libcxx/test/std/ranges/range.utility/range.subrange/types.h @@ -9,6 +9,11 @@ #ifndef LIBCXX_TEST_STD_RANGES_RANGE_UTILITY_RANGE_SUBRANGE_TYPES_H #define LIBCXX_TEST_STD_RANGES_RANGE_UTILITY_RANGE_SUBRANGE_TYPES_H +#include +#include +#include +#include + #include "test_macros.h" #include "test_iterators.h" @@ -61,12 +66,12 @@ struct SizedSentinelForwardIter { typedef std::make_unsigned_t udifference_type; typedef SizedSentinelForwardIter self; - int *base; - SizedSentinelForwardIter() = default; - constexpr SizedSentinelForwardIter(int *ptr) : base(ptr) { } + constexpr explicit SizedSentinelForwardIter(int *ptr, bool *minusWasCalled) + : base_(ptr), minusWasCalled_(minusWasCalled) + { } - friend constexpr bool operator==(const self& lhs, const self& rhs) { return lhs.base == rhs.base; } + friend constexpr bool operator==(const self& lhs, const self& rhs) { return lhs.base_ == rhs.base_; } reference operator*() const; pointer operator->() const; @@ -75,16 +80,20 @@ struct SizedSentinelForwardIter { self& operator--(); self operator--(int); - static int minusCount; - friend constexpr difference_type operator-(SizedSentinelForwardIter const&a, - SizedSentinelForwardIter const&b) { - if (!std::is_constant_evaluated()) - minusCount++; - return a.base - b.base; + friend constexpr difference_type operator-(SizedSentinelForwardIter const& a, + SizedSentinelForwardIter const& b) { + if (a.minusWasCalled_) + *a.minusWasCalled_ = true; + if (b.minusWasCalled_) + *b.minusWasCalled_ = true; + return a.base_ - b.base_; } -}; -int SizedSentinelForwardIter::minusCount = 0; +private: + int *base_ = nullptr; + bool *minusWasCalled_ = nullptr; +}; +static_assert(std::sized_sentinel_for); struct ConvertibleForwardIter { typedef std::forward_iterator_tag iterator_category;