[libcxx-commits] [libcxx] 86df5a2 - [libc++] Simplify std::ranges::subrange

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Tue Sep 28 14:34:07 PDT 2021


Author: Louis Dionne
Date: 2021-09-28T17:34:01-04:00
New Revision: 86df5a2fa832cdb42928c7a6670eb927f04b98d4

URL: https://github.com/llvm/llvm-project/commit/86df5a2fa832cdb42928c7a6670eb927f04b98d4
DIFF: https://github.com/llvm/llvm-project/commit/86df5a2fa832cdb42928c7a6670eb927f04b98d4.diff

LOG: [libc++] Simplify std::ranges::subrange

Instead of using a base class to store the members and the optional
size, use [[no_unique_address]] to achieve the same thing without
needing a base class.

Also, as a fly-by:
- Change subrange from struct to class (per the standard)
- Improve the diagnostic for when one doesn't provide a size to the ctor of a sized subrange
- Replace this->member by just member since it's not in a dependent base anymore

This change would be an ABI break due to [[no_unique_address]], but we
haven't shipped ranges anywhere yet, so this shouldn't affect anyone.

Differential Revision: https://reviews.llvm.org/D110370

Added: 
    libcxx/test/std/ranges/range.utility/range.subrange/ctor.default.pass.cpp

Modified: 
    libcxx/include/__ranges/subrange.h

Removed: 
    


################################################################################
diff  --git a/libcxx/include/__ranges/subrange.h b/libcxx/include/__ranges/subrange.h
index f42fdd5ae8236..af4e27600625d 100644
--- a/libcxx/include/__ranges/subrange.h
+++ b/libcxx/include/__ranges/subrange.h
@@ -67,62 +67,41 @@ namespace ranges {
 
   enum class _LIBCPP_ENUM_VIS subrange_kind : bool { unsized, sized };
 
-  template<class _Iter, class _Sent, bool>
-  struct __subrange_base {
-    static constexpr bool __store_size = false;
-    _Iter __begin_ = _Iter();
-    _Sent __end_ = _Sent();
-
-    _LIBCPP_HIDE_FROM_ABI
-    constexpr __subrange_base() = default;
-
-    _LIBCPP_HIDE_FROM_ABI
-    constexpr __subrange_base(_Iter __iter, _Sent __sent, make_unsigned_t<iter_
diff erence_t<_Iter>> = 0)
-      : __begin_(_VSTD::move(__iter)), __end_(__sent) { }
-  };
-
-  template<class _Iter, class _Sent>
-  struct __subrange_base<_Iter, _Sent, true> {
-    static constexpr bool __store_size = true;
-    _Iter __begin_ = _Iter();
-    _Sent __end_ = _Sent();
-    make_unsigned_t<iter_
diff erence_t<_Iter>> __size_ = 0;
-
-    _LIBCPP_HIDE_FROM_ABI
-    constexpr __subrange_base() = default;
-
-    _LIBCPP_HIDE_FROM_ABI
-    constexpr __subrange_base(_Iter __iter, _Sent __sent, decltype(__size_) __size)
-      : __begin_(_VSTD::move(__iter)), __end_(__sent), __size_(__size) { }
-  };
-
   template<input_or_output_iterator _Iter, sentinel_for<_Iter> _Sent = _Iter,
            subrange_kind _Kind = sized_sentinel_for<_Sent, _Iter>
              ? subrange_kind::sized
              : subrange_kind::unsized>
     requires (_Kind == subrange_kind::sized || !sized_sentinel_for<_Sent, _Iter>)
-  struct _LIBCPP_TEMPLATE_VIS subrange
-    : public view_interface<subrange<_Iter, _Sent, _Kind>>,
-      private __subrange_base<_Iter, _Sent, _Kind == subrange_kind::sized && !sized_sentinel_for<_Sent, _Iter>> {
-
-    using _Base = __subrange_base<_Iter, _Sent, _Kind == subrange_kind::sized && !sized_sentinel_for<_Sent, _Iter>>;
-
+  class _LIBCPP_TEMPLATE_VIS subrange
+    : public view_interface<subrange<_Iter, _Sent, _Kind>>
+  {
+  private:
+    static constexpr bool _StoreSize = (_Kind == subrange_kind::sized && !sized_sentinel_for<_Sent, _Iter>);
+    static constexpr bool _MustProvideSizeAtConstruction = !_StoreSize; // just to improve compiler diagnostics
+    struct _Empty { constexpr _Empty(auto) noexcept { } };
+    using _Size = conditional_t<_StoreSize, make_unsigned_t<iter_
diff erence_t<_Iter>>, _Empty>;
+    [[no_unique_address]] _Iter __begin_ = _Iter();
+    [[no_unique_address]] _Sent __end_ = _Sent();
+    [[no_unique_address]] _Size __size_ = 0;
+
+  public:
     _LIBCPP_HIDE_FROM_ABI
     subrange() requires default_initializable<_Iter> = default;
 
     _LIBCPP_HIDE_FROM_ABI
     constexpr subrange(__convertible_to_non_slicing<_Iter> auto __iter, _Sent __sent)
-      requires (!_Base::__store_size)
-      : _Base(_VSTD::move(__iter), __sent) {}
+      requires _MustProvideSizeAtConstruction
+      : __begin_(_VSTD::move(__iter)), __end_(std::move(__sent))
+    { }
 
     _LIBCPP_HIDE_FROM_ABI
     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)
+      : __begin_(_VSTD::move(__iter)), __end_(std::move(__sent)), __size_(__n)
     {
       if constexpr (sized_sentinel_for<_Sent, _Iter>)
-        _LIBCPP_ASSERT((this->__end_ - this->__begin_) == static_cast<iter_
diff erence_t<_Iter>>(__n),
+        _LIBCPP_ASSERT((__end_ - __begin_) == static_cast<iter_
diff erence_t<_Iter>>(__n),
           "std::ranges::subrange was passed an invalid size hint");
     }
 
@@ -132,8 +111,9 @@ namespace ranges {
                convertible_to<sentinel_t<_Range>, _Sent>
     _LIBCPP_HIDE_FROM_ABI
     constexpr subrange(_Range&& __range)
-      requires (!_Base::__store_size)
-      : subrange(ranges::begin(__range), ranges::end(__range)) { }
+      requires (!_StoreSize)
+      : subrange(ranges::begin(__range), ranges::end(__range))
+    { }
 
     template<__
diff erent_from<subrange> _Range>
       requires borrowed_range<_Range> &&
@@ -141,9 +121,9 @@ namespace ranges {
                convertible_to<sentinel_t<_Range>, _Sent>
     _LIBCPP_HIDE_FROM_ABI
     constexpr subrange(_Range&& __range)
-      requires _Base::__store_size && sized_range<_Range>
-      : subrange(__range, ranges::size(__range)) { }
-
+      requires _StoreSize && sized_range<_Range>
+      : subrange(__range, ranges::size(__range))
+    { }
 
     template<borrowed_range _Range>
       requires __convertible_to_non_slicing<iterator_t<_Range>, _Iter> &&
@@ -151,39 +131,47 @@ namespace ranges {
     _LIBCPP_HIDE_FROM_ABI
     constexpr subrange(_Range&& __range, make_unsigned_t<iter_
diff erence_t<_Iter>> __n)
       requires (_Kind == subrange_kind::sized)
-      : subrange(ranges::begin(__range), ranges::end(__range), __n) { }
+      : subrange(ranges::begin(__range), ranges::end(__range), __n)
+    { }
 
     template<__
diff erent_from<subrange> _Pair>
       requires __pair_like_convertible_from<_Pair, const _Iter&, const _Sent&>
     _LIBCPP_HIDE_FROM_ABI
-    constexpr operator _Pair() const { return _Pair(this->__begin_, this->__end_); }
+    constexpr operator _Pair() const {
+      return _Pair(__begin_, __end_);
+    }
 
     _LIBCPP_HIDE_FROM_ABI
     constexpr _Iter begin() const requires copyable<_Iter> {
-      return this->__begin_;
+      return __begin_;
     }
 
     [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Iter begin() requires (!copyable<_Iter>) {
-      return _VSTD::move(this->__begin_);
+      return _VSTD::move(__begin_);
     }
 
     _LIBCPP_HIDE_FROM_ABI
-    constexpr _Sent end() const { return this->__end_; }
+    constexpr _Sent end() const {
+      return __end_;
+    }
 
-    [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr bool empty() const { return this->__begin_ == this->__end_; }
+    [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr bool empty() const {
+      return __begin_ == __end_;
+    }
 
     _LIBCPP_HIDE_FROM_ABI
     constexpr make_unsigned_t<iter_
diff erence_t<_Iter>> size() const
       requires (_Kind == subrange_kind::sized)
     {
-      if constexpr (_Base::__store_size)
-        return this->__size_;
+      if constexpr (_StoreSize)
+        return __size_;
       else
-        return __to_unsigned_like(this->__end_ - this->__begin_);
+        return _VSTD::__to_unsigned_like(__end_ - __begin_);
     }
 
     [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr subrange next(iter_
diff erence_t<_Iter> __n = 1) const&
-      requires forward_iterator<_Iter> {
+      requires forward_iterator<_Iter>
+    {
       auto __tmp = *this;
       __tmp.advance(__n);
       return __tmp;
@@ -195,7 +183,8 @@ namespace ranges {
     }
 
     [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr subrange prev(iter_
diff erence_t<_Iter> __n = 1) const
-      requires bidirectional_iterator<_Iter> {
+      requires bidirectional_iterator<_Iter>
+    {
       auto __tmp = *this;
       __tmp.advance(-__n);
       return __tmp;
@@ -205,16 +194,16 @@ namespace ranges {
     constexpr subrange& advance(iter_
diff erence_t<_Iter> __n) {
       if constexpr (bidirectional_iterator<_Iter>) {
         if (__n < 0) {
-          ranges::advance(this->__begin_, __n);
-          if constexpr (_Base::__store_size)
-            this->__size_ += _VSTD::__to_unsigned_like(-__n);
+          ranges::advance(__begin_, __n);
+          if constexpr (_StoreSize)
+            __size_ += _VSTD::__to_unsigned_like(-__n);
           return *this;
         }
       }
 
-      auto __d = __n - ranges::advance(this->__begin_, __n, this->__end_);
-      if constexpr (_Base::__store_size)
-        this->__size_ -= _VSTD::__to_unsigned_like(__d);
+      auto __d = __n - ranges::advance(__begin_, __n, __end_);
+      if constexpr (_StoreSize)
+        __size_ -= _VSTD::__to_unsigned_like(__d);
       return *this;
     }
   };

diff  --git a/libcxx/test/std/ranges/range.utility/range.subrange/ctor.default.pass.cpp b/libcxx/test/std/ranges/range.utility/range.subrange/ctor.default.pass.cpp
new file mode 100644
index 0000000000000..74840a4285d9e
--- /dev/null
+++ b/libcxx/test/std/ranges/range.utility/range.subrange/ctor.default.pass.cpp
@@ -0,0 +1,69 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: libcpp-no-concepts
+// UNSUPPORTED: libcpp-has-no-incomplete-ranges
+
+// constexpr subrange() requires default_initializable<I>;
+
+#include <ranges>
+
+#include <cassert>
+#include <cstddef>
+
+#include "test_iterators.h"
+
+// An input_or_output_iterator that is not default constructible so we can test
+// the `requires` on subrange's default constructor.
+struct NoDefaultIterator {
+  using 
diff erence_type = std::ptr
diff _t;
+  NoDefaultIterator() = delete;
+  NoDefaultIterator& operator++();
+  void operator++(int);
+  int& operator*() const;
+  friend bool operator==(NoDefaultIterator const&, NoDefaultIterator const&);
+};
+static_assert(std::input_or_output_iterator<NoDefaultIterator>);
+
+// A sentinel type for the above iterator
+struct Sentinel {
+  friend bool operator==(NoDefaultIterator const&, Sentinel const&);
+  friend bool operator==(Sentinel const&, NoDefaultIterator const&);
+  friend bool operator!=(NoDefaultIterator const&, Sentinel const&);
+  friend bool operator!=(Sentinel const&, NoDefaultIterator const&);
+};
+
+constexpr bool test() {
+  {
+    static_assert(!std::is_default_constructible_v<std::ranges::subrange<NoDefaultIterator, Sentinel, std::ranges::subrange_kind::sized>>);
+    static_assert(!std::is_default_constructible_v<std::ranges::subrange<NoDefaultIterator, Sentinel, std::ranges::subrange_kind::unsized>>);
+  }
+
+  {
+    using Iter = forward_iterator<int*>;
+    std::ranges::subrange<Iter, Iter, std::ranges::subrange_kind::sized> subrange;
+    assert(subrange.begin() == Iter());
+    assert(subrange.end() == Iter());
+  }
+  {
+    using Iter = forward_iterator<int*>;
+    std::ranges::subrange<Iter, Iter, std::ranges::subrange_kind::unsized> subrange;
+    assert(subrange.begin() == Iter());
+    assert(subrange.end() == Iter());
+  }
+
+  return true;
+}
+
+int main(int, char**) {
+  test();
+  static_assert(test());
+
+  return 0;
+}


        


More information about the libcxx-commits mailing list