[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 (file)
--- 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<iter_difference_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_difference_t<_Iter>>(__n),
+          "std::ranges::subrange was passed an invalid size hint");
+    }
 
     template<__different_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 5cd6d95..b2f135e 100644 (file)
--- 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 8dfae99..c07bfff 100644 (file)
--- 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::ptrdiff_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<SizedSentinelForwardIter, SizedSentinelForwardIter>);
 
 struct ConvertibleForwardIter {
     typedef std::forward_iterator_tag       iterator_category;