[libcxx-commits] [libcxx] fd66b44 - [libc++] Add an assertion in the subrange constructors with a size hint
Louis Dionne via libcxx-commits
libcxx-commits at lists.llvm.org
Fri Sep 3 13:04:16 PDT 2021
Author: Louis Dionne
Date: 2021-09-03T16:04:02-04:00
New Revision: fd66b44ec19e76c111c8a37a42dcb4b44a96bfd6
URL: https://github.com/llvm/llvm-project/commit/fd66b44ec19e76c111c8a37a42dcb4b44a96bfd6
DIFF: https://github.com/llvm/llvm-project/commit/fd66b44ec19e76c111c8a37a42dcb4b44a96bfd6.diff
LOG: [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
Added:
Modified:
libcxx/include/__ranges/subrange.h
libcxx/test/std/ranges/range.utility/range.subrange/primitives.pass.cpp
libcxx/test/std/ranges/range.utility/range.subrange/types.h
Removed:
################################################################################
diff --git a/libcxx/include/__ranges/subrange.h b/libcxx/include/__ranges/subrange.h
index e162d6b316952..f42fdd5ae8236 100644
--- a/libcxx/include/__ranges/subrange.h
+++ b/libcxx/include/__ranges/subrange.h
@@ -15,10 +15,11 @@
#include <__concepts/derived_from.h>
#include <__concepts/
diff erent_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<iter_
diff erence_t<_Iter>> __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<iter_
diff erence_t<_Iter>>(__n),
+ "std::ranges::subrange was passed an invalid size hint");
+ }
template<__
diff erent_from<subrange> _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 5cd6d95b761f2..b2f135e2e159d 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 <ranges>
#include <cassert>
-#include "test_macros.h"
#include "test_iterators.h"
#include "types.h"
constexpr bool test() {
- std::ranges::subrange<MoveOnlyForwardIter, int*> 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<ForwardIter> b(ForwardIter(nullptr), ForwardIter(nullptr));
- assert(b.empty());
+ {
+ std::ranges::subrange<MoveOnlyForwardIter, int*> a(MoveOnlyForwardIter(buff), buff + 5, 5);
+ assert(a.begin().base == buff);
+ assert(!a.empty());
+ assert(a.size() == 5);
+ }
- std::ranges::subrange<ForwardIter> c{ForwardIter(globalBuff), ForwardIter(globalBuff)};
- assert(c.empty());
+ {
+ std::ranges::subrange<ForwardIter> b(ForwardIter(nullptr), ForwardIter(nullptr));
+ assert(b.empty());
+ }
- std::ranges::subrange<ForwardIter> d(ForwardIter(globalBuff), ForwardIter(globalBuff + 1));
- assert(!d.empty());
+ {
+ std::ranges::subrange<ForwardIter> c{ForwardIter(buff), ForwardIter(buff)};
+ assert(c.empty());
+ }
- std::ranges::subrange<SizedSentinelForwardIter> e(SizedSentinelForwardIter(globalBuff),
- SizedSentinelForwardIter(globalBuff + 8), 8);
- assert(!e.empty());
- assert(e.size() == 8);
+ {
+ std::ranges::subrange<ForwardIter> 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<SizedSentinelForwardIter> 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 8dfae99c62459..c07bfffe82105 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 <cstddef>
+#include <iterator>
+#include <ranges>
+#include <type_traits>
+
#include "test_macros.h"
#include "test_iterators.h"
@@ -61,12 +66,12 @@ struct SizedSentinelForwardIter {
typedef std::make_unsigned_t<std::ptr
diff _t> u
diff erence_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
diff erence_type operator-(SizedSentinelForwardIter const&a,
- SizedSentinelForwardIter const&b) {
- if (!std::is_constant_evaluated())
- minusCount++;
- return a.base - b.base;
+ friend constexpr
diff erence_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<SizedSentinelForwardIter, SizedSentinelForwardIter>);
struct ConvertibleForwardIter {
typedef std::forward_iterator_tag iterator_category;
More information about the libcxx-commits
mailing list