reuse cartesian product utils to define concat-is-bidirectional

concat.lwg.feedback.20230906.md

@@ -27,7 +27,7 @@
        “Exception specification is equivalent to:
        noexcept(ranges::swap(*x, *y)) && ... && noexcept(ranges::iter_swap(its, its))
        where its is a pack of lvalues of type iterator_t<maybe-const<Const, Views>> const respectively.”
-[TODO] TK: on concat-is-bidirectional: we check it twice
+[Done] TK: on concat-is-bidirectional: we check it twice
        TK: we have similar things in the cartesian_view: we could extract the logic for cartesian-is-common
        JW: that makes sense
 

concat.md

@@ -1,8 +1,8 @@
 ---
 title: "`views::concat`"
-document: P2542R3
-date: 2023-06-07
-audience: SG9, LEWG
+document: P2542R5
+date: 2023-09-12
+audience: SG9, LEWG, LWG
 author:
   - name: Hui Xie
     email: <[email protected]>
@@ -13,6 +13,14 @@ toc: true
 
 # Revision History
 
+## R5
+
+- Fix `static_cast<difference_type>`
+- Reuse utilities in `cartesian_product_view` to define `concat-is-bidirectional`
+- Various wording fixes
+## R4
+
+- Add `concat_expert`
 ## R3
 
 - Redesigned `iter_swap`
@@ -583,6 +591,54 @@ namespace std::ranges {
 }
 ```
 
+Add the following to [range.adaptor.helpers]{.sref}
+
+```diff
+namespace std::ranges {
+// [...]
++ template<class R>
++   concept @*common-arg*@ =                        // exposition only
++     common_range<R> || (sized_range<R> && random_access_range<R>);
+// [...]
+}
+```
+
+Modify the following in [range.cartesian.view]{.sref}
+
+```diff
+namespace std::ranges {
+// [...]
+- template<class R>
+-   concept @*cartesian-product-common-arg*@ =      // exposition only
+-     common_range<R> || (sized_range<R> && random_access_range<R>);
+
+  template<bool Const, class First, class... Vs>
+  concept @*cartesian-product-is-bidirectional*@ =          // exposition only
+    (bidirectional_range<@*maybe-const*@<Const, First>> && ... &&
+      (bidirectional_range<@*maybe-const*@<Const, Vs>>
+-        && @*cartesian-product-common-arg*@<@*maybe-const*@<Const, Vs>>));
++        && @*common-arg*@<@*maybe-const*@<Const, Vs>>));
+
+  template<class First, class... Vs>
+  concept @*cartesian-product-is-common*@ =                 // exposition only
+-    @*cartesian-product-common-arg*@<First>;
++    @*common-arg*@<First>;
+
+// [...]
+
+-  template<@*cartesian-product-common-arg*@ R>
++  template<@*common-arg*@ R>
+  constexpr auto @*cartesian-common-arg-end*@(R& r) {       // exposition only
+    if constexpr (common_range<R>) {
+      return ranges::end(r);
+    } else {
+      return ranges::begin(r) + ranges::distance(r);
+    }
+  }
+// [...]
+}
+```
+
 ## `concat`
 
 Add the following subclause to [range.adaptors]{.sref}.
@@ -636,11 +692,6 @@ namespace std::ranges {
   template <bool Const, class... Rs>
   concept @_concat-is-random-access_@ = @*see below*@;   // exposition only
 
-  template <class R>
-  concept @_constant-time-reversible_@ =          // exposition only
-    (bidirectional_range<R> && common_range<R>) ||
-    (sized_range<R> && random_access_range<R>);
-
   template <bool Const, class... Rs>
   concept @_concat-is-bidirectional_@ = @*see below*@;   // exposition only
 
@@ -756,12 +807,12 @@ concept @_concat-is-bidirectional_@ = @*see below*@; // exposition only
 
 :::bq
 
-[4]{.pnum} Let `V` be the last element of `Rs`, and `Fs` be the pack that consists of all elements of `Rs` except `V`, then `@_concat-is-bidirectional_@` is equivalent to:
+[4]{.pnum} Let `Fs` be the pack that consists of all elements of `Rs` except the last element, then `@_concat-is-bidirectional_@` is equivalent to:
 
 ```cpp
 template <bool Const, class... Rs>
 concept @_concat-is-bidirectional_@ = // exposition only
-  (bidirectional_range<@*maybe_const*@<Const, V>> && ... && @*constant-time-reversible*@<@*maybe_const*@<Const, Fs>>);
+  (@*all-bidirectional*@<Const, Rs...> && ... && @*common-arg*@<@*maybe_const*@<Const, Fs>>);
 ```
 
 :::

impl/concat/concat.hpp

@@ -25,6 +25,10 @@ concept all_bidirectional = (bidirectional_range<__maybe_const<Const, Views>> &&
 template <bool Const, class... Views>
 concept all_forward = (forward_range<__maybe_const<Const, Views>> && ...);
 
+template<class R>
+concept common_arg =                // exposition only
+  common_range<R> || (sized_range<R> && random_access_range<R>);
+
 inline namespace not_to_spec {
 
 template <class... Rs>
@@ -86,14 +90,9 @@ concept concat_is_random_access =
     (all_random_access<Const, Views> && ...) &&
     (all_but_last<sized_range<__maybe_const<Const, Views>>...>);
 
-template <class R>
-concept constant_time_reversible = (bidirectional_range<R> &&
-                                    common_range<R>) ||
-                                   (sized_range<R> && random_access_range<R>);
-
-template <class... Rs>
-concept concat_bidirectional = all_but_last<constant_time_reversible<Rs>...> &&
-    bidirectional_range<back<Rs...>>;
+template <bool Const, class... Views>
+concept concat_bidirectional = all_but_last<common_arg<__maybe_const<Const, Views>>...> &&
+    all_bidirectional<Const, Views...>;
 
 static_assert(true);  // clang-format badness
 
@@ -161,7 +160,7 @@ template <bool Const, class... Ts>
 consteval auto iterator_concept_test() {
   if constexpr (concat_is_random_access<Const, Ts...>) {
     return random_access_iterator_tag{};
-  } else if constexpr (concat_bidirectional<__maybe_const<Const, Ts>...>) {
+  } else if constexpr (concat_bidirectional<Const, Ts...>) {
     return bidirectional_iterator_tag{};
   } else if constexpr ((forward_range<__maybe_const<Const, Ts>> && ...)) {
     return forward_iterator_tag{};
@@ -212,7 +211,7 @@ consteval auto iter_cat_test() {
   } else if constexpr ((has_tag<bidirectional_iterator_tag,
                                 __maybe_const<Const, Views>> &&
                         ...) &&
-                       concat_bidirectional<__maybe_const<Const, Views>...>) {
+                       concat_bidirectional<Const, Views...>) {
     return bidirectional_iterator_tag{};
   } else if constexpr ((has_tag<forward_iterator_tag,
                                 __maybe_const<Const, Views>> &&
@@ -404,13 +403,13 @@ class concat_view : public view_interface<concat_view<Views...>> {
     }
 
     constexpr iterator& operator--() requires
-        xo::concat_bidirectional<__maybe_const<Const, Views>...> {
+        xo::concat_bidirectional<Const, Views...> {
       xo::visit_i(it_, [this](auto I, auto&&) { this->prev<I>(); });
       return *this;
     }
 
     constexpr iterator operator--(
-        int) requires xo::concat_bidirectional<__maybe_const<Const, Views>...> {
+        int) requires xo::concat_bidirectional<Const, Views...> {
       auto tmp = *this;
       --*this;
       return tmp;

impl/concat/test/basics.cpp

@@ -291,9 +291,9 @@ TEST_POINT("bidirectional_concept") {
 
     static_assert(!common_range<decltype(nonCommonBidirRange)>);
     static_assert(bidirectional_range<decltype(nonCommonBidirRange)>);
-    STATIC_REQUIRE(xo::constant_time_reversible<decltype(v1)>);                   // random
-    STATIC_REQUIRE(xo::constant_time_reversible<decltype(l1)>);                   // common
-    STATIC_REQUIRE(!xo::constant_time_reversible<decltype(nonCommonBidirRange)>); // bidir only
+   // STATIC_REQUIRE(xo::constant_time_reversible<decltype(v1)>);                   // random
+   // STATIC_REQUIRE(xo::constant_time_reversible<decltype(l1)>);                   // common
+   // STATIC_REQUIRE(!xo::constant_time_reversible<decltype(nonCommonBidirRange)>); // bidir only
 
     STATIC_CHECK(bidirectional_range<decltype(views::concat(v1, nonCommonBidirRange))>);
 }