3820. cartesian_product_view::iterator::prev is not quite right

Section: 25.7.33.3 [range.cartesian.iterator] Status: C++23 Submitter: Hewill Kang Opened: 2022-11-08 Last modified: 2023-11-22

Priority: Not Prioritized

View all other issues in [range.cartesian.iterator].

View all issues with C++23 status.

Discussion:

Currently, cartesian_product_view::iterator::prev has the following Effects:

auto& it = std::get<N>(current_);
if (it == ranges::begin(std::get<N>(parent_->bases_))) {
  it = cartesian-common-arg-end(std::get<N>(parent_->bases_));
  if constexpr (N > 0) {
    prev<N - 1>();
  }
}
--it;

which decrements the underlying iterator one by one using recursion. However, when N == 0, it still detects if the first iterator has reached the beginning and assigns it to the end, which is not only unnecessary, but also causes cartesian-common-arg-end to be applied to the first range, making it ill-formed in some cases, for example:

#include <ranges>

int main() {
  auto r = std::views::cartesian_product(std::views::iota(0));
  r.begin() += 3; // hard error
}

This is because, for the first range, cartesian_product_view::iterator::operator+= only requires it to model random_access_range. However, when x is negative, this function will call prev and indirectly calls cartesian-common-arg-end, since the latter constrains its argument to satisfy cartesian-product-common-arg, that is, common_range<R> || (sized_range<R> && random_access_range<R>), which is not the case for the unbounded iota_view, resulting in a hard error in prev's function body.

The proposed resolution changes the position of the if constexpr so that we just decrement the first iterator and nothing else.

[Kona 2022-11-08; Move to Ready]

[2023-02-13 Approved at February 2023 meeting in Issaquah. Status changed: Voting → WP.]

Proposed resolution:

This wording is relative to N4917.

1. Modify [ranges.cartesian.iterator] as indicated:

   template<size_t N = sizeof...(Vs)>
     constexpr void prev();
   

   -6- Effects: Equivalent to:

   auto& it = std::get<N>(current_);
   if constexpr (N > 0) {
     if (it == ranges::begin(std::get<N>(parent_->bases_))) {
       it = cartesian-common-arg-end(std::get<N>(parent_->bases_));
       if constexpr (N > 0) {
         prev<N - 1>();
       }
     }
   }
   --it;