3472. counted_iterator is missing preconditions

Section: 24.5.7 [iterators.counted] Status: C++23 Submitter: Michael Schellenberger Costa Opened: 2020-07-29 Last modified: 2023-11-22

Priority: Not Prioritized

View all issues with C++23 status.

Discussion:

C++20 introduces a new iterator counted_iterator that keeps track of the end of its range via an additional exposition only member length.

Consequently, there are several preconditions for many member functions of counted_iterator, but it seems some are missing:

1. operator*

   Here we have no precondition regarding length. However, given that length denotes the distance to the end of the range it should be invalid to dereference a counted_iterator with length 0.

   Moreover, operator[] has a precondition of "n < length". Consider the following code snippet:

   int some_ints[] = {0,1,2};
   counted_iterator<int*> i{some_ints, 0};
   

   Here "i[0]" would be invalid due to the precondition "n < length". However, "*i" would be a valid expression. This violates the definition of operator[] which states according to 7.6.1.2 [expr.sub] p1:

   […] The expression E1[E2] is identical (by definition) to *((E1)+(E2)) […]

   Substituting E2->0 we get

   […] The expression E1[0] is identical (by definition) to *(E1) […]

   With the current wording counted_iterator violates that definition and we should add to operator*:

   Preconditions: length > 0.

2. iter_move

   This is a similar case. We have only the Effects element:

   Effects: Equivalent to: return ranges::iter_move(i.current);

   However, looking at the requirements of ranges::iter_move we have in 24.3.3.1 [iterator.cust.move] p2:

   If ranges::iter_move(E) is not equal to *E, the program is ill-formed, no diagnostic required.

   This clearly requires that for counted_iterator::iter_move to be well-formed, we need counted_iterator::operator* to be well formed. Consequently we should also add the same precondition to counted_iterator::iter_move:

   Preconditions: length > 0.

3. iter_swap

   This is essentially the same arguing as for counted_iterator::iter_move. The essential observation is that ranges::iter_swap is defined in terms of ranges::iter_move (see 24.3.3.2 [iterator.cust.swap]) so it must have the same preconditions and we should add:

   Preconditions: length > 0.

[2020-08-21 Issue processing telecon: moved to Tentatively Ready.]

[2020-11-09 Approved In November virtual meeting. Status changed: Tentatively Ready → WP.]

Proposed resolution:

This wording is relative to N4861.

1. Modify 24.5.7.4 [counted.iter.elem] as indicated:

   constexpr decltype(auto) operator*();
   constexpr decltype(auto) operator*() const
     requires dereferenceable<const I>;
   

   -?- Preconditions: length > 0.

   -1- Effects: Equivalent to: return *current;

2. Modify 24.5.7.7 [counted.iter.cust] as indicated:

   friend constexpr iter_rvalue_reference_t<I>
     iter_move(const counted_iterator& i)
       noexcept(noexcept(ranges::iter_move(i.current)))
       requires input_iterator<I>;
   

   -?- Preconditions: i.length > 0.

   -1- Effects: Equivalent to: return ranges::iter_move(i.current);

   template<indirectly_swappable<I> I2>
     friend constexpr void
       iter_swap(const counted_iterator& x, const counted_iterator<I2>& y)
         noexcept(noexcept(ranges::iter_swap(x.current, y.current)));
   

   -?- Preconditions: x.length > 0 and y.length > 0.

   -1- Effects: Equivalent to: return ranges::iter_swap(x.current, y.current);