feature/spaceship: Clause 23: Iterators

[StephanTLavavej]

Works towards #62.

@CaseyCarter and I audited WG21-P1614's changes to Clause 23 Iterators, and found that everything was already implemented (in main, not just feature/spaceship) with the exception of operator!= removal for istream_iterator and istreambuf_iterator. I've implemented those removals for C++20 mode.

In general, we believe that operator!= removal doesn't merit the addition of test coverage - we should already have been testing !=, and now the compiler will rewrite it for us. In this case, we need to skip a libcxx test, as they were directly calling std::operator!= which this C++20 feature intentionally makes into an error.

Two notes from our investigation:

(1) unreachable_sentinel_t

WG21-N4878 [unreachable.sentinel]/2 depicts a by-value parameter:

struct unreachable_sentinel_t {
  template<weakly_incrementable I>
    friend constexpr bool operator==(unreachable_sentinel_t, const I&) noexcept
    { return false; }
};

while we implement a by-reference parameter:

STL/stl/inc/xutility

Line 4014 in 7f08bb3

_NODISCARD friend constexpr bool operator==(const unreachable_sentinel_t&, const _Winc&) noexcept {

The difference is essentially unobservable, and a significant restructuring would be required in order to take unreachable_sentinel_t by value while preserving our compiler throughput workaround for MSVC permissive mode - not worth it.

(2) common_iterator

WG21-N4878 [common.iter.cmp] depicts two overloads:

template<class I2, sentinel_for<I> S2>
  requires sentinel_for<S, I2>
friend bool operator==(
  const common_iterator& x, const common_iterator<I2, S2>& y);

template<class I2, sentinel_for<I> S2>
  requires sentinel_for<S, I2> && equality_comparable_with<I, I2>
friend bool operator==(
  const common_iterator& x, const common_iterator<I2, S2>& y);

while we implement one overload:

STL/stl/inc/iterator

Lines 937 to 964 in 7f08bb3

	template <class _OIter, sentinel_for<_Iter> _OSe>
	requires sentinel_for<_Se, _OIter>
	_NODISCARD friend bool operator==(const common_iterator& _Left, const common_iterator<_OIter, _OSe>& _Right) {
	// clang-format on
	#if _ITERATOR_DEBUG_LEVEL != 0
	_STL_VERIFY(
	_Left._Val._Contains != _Variantish_state::_Nothing && _Right._Val._Contains != _Variantish_state::_Nothing,
	"common_iterators can only be compared if both hold a value");
	#endif // _ITERATOR_DEBUG_LEVEL != 0
	if (_Left._Val._Contains == _Variantish_state::_Holds_iter) {
	if (_Right._Val._Contains == _Variantish_state::_Holds_iter) {
	if constexpr (equality_comparable_with<_Iter, _OIter>) {
	return _Left._Val._Iterator == _Right._Val._Iterator;
	} else {
	return true;
	}
	} else {
	return _Left._Val._Iterator == _Right._Val._Sentinel;
	}
	} else {
	if (_Right._Val._Contains == _Variantish_state::_Holds_iter) {
	return _Left._Val._Sentinel == _Right._Val._Iterator;
	} else {
	return true;
	}
	}
	}

This is conformant, as the if constexpr (equality_comparable_with<_Iter, _OIter>) implements the behavior of the two-overload set, with far less code repetition and compiler throughput impact.

[miscco]

Unsurprising I could not even find a nitpick

[CaseyCarter]

(2) common_iterator

WG21-N4878 [common.iter.cmp] depicts two overloads:

template<class I2, sentinel_for<I> S2>
  requires sentinel_for<S, I2>
friend bool operator==(
  const common_iterator& x, const common_iterator<I2, S2>& y);

template<class I2, sentinel_for<I> S2>
  requires sentinel_for<S, I2> && equality_comparable_with<I, I2>
friend bool operator==(
  const common_iterator& x, const common_iterator<I2, S2>& y);

while we implement one overload:

STL/stl/inc/iterator

Lines 937 to 964 in 7f08bb3

	template <class _OIter, sentinel_for<_Iter> _OSe>
	requires sentinel_for<_Se, _OIter>
	_NODISCARD friend bool operator==(const common_iterator& _Left, const common_iterator<_OIter, _OSe>& _Right) {
	// clang-format on
	#if _ITERATOR_DEBUG_LEVEL != 0
	_STL_VERIFY(
	_Left._Val._Contains != _Variantish_state::_Nothing && _Right._Val._Contains != _Variantish_state::_Nothing,
	"common_iterators can only be compared if both hold a value");
	#endif // _ITERATOR_DEBUG_LEVEL != 0
	if (_Left._Val._Contains == _Variantish_state::_Holds_iter) {
	if (_Right._Val._Contains == _Variantish_state::_Holds_iter) {
	if constexpr (equality_comparable_with<_Iter, _OIter>) {
	return _Left._Val._Iterator == _Right._Val._Iterator;
	} else {
	return true;
	}
	} else {
	return _Left._Val._Iterator == _Right._Val._Sentinel;
	}
	} else {
	if (_Right._Val._Contains == _Variantish_state::_Holds_iter) {
	return _Left._Val._Sentinel == _Right._Val._Iterator;
	} else {
	return true;
	}
	}
	}

This is conformant, as the if constexpr (equality_comparable_with<_Iter, _OIter>) implements the behavior of the two-overload set, with far less code repetition and compiler throughput impact.

This is an idiomatic transformation when working with concept-constrained function templates. When two overloads are identical modulo constraints, and one set of constraints subsumes the other, we can merge those overloads unobservably. Assume we have:

template <class T>
  requires A<T>
void f(const T&) { stuff(); }

template <class T>
  requires A<T> && B<T>
void f(const T&) { other_stuff(); }

for example. These are equivalent to:

template <class T>
  requires A<T>
void f(const T&) {
  if constexpr (B<T>) {
    other_stuff();
  } else {
    stuff();
  }
}

since in both cases we:

• evaluate other_stuff() when both A<T> and B<T> hold,
• evaluate stuff() when only A<T> holds, and
• reject argument types for which A<T> is not true.

This is a huge boon to throughput since subsumption checking, as would be necessary for overload resolution in the original case when both A<T> and B<T> are satisfied, can be quite expensive. If both of our original overloads have conditional noexcept, however:

template <class T>
  requires A<T>
void f(const T&) noexcept(noexcept(stuff())) { stuff(); }

template <class T>
  requires A<T> && B<T>
void f(const T&) noexcept(noexcept(other_stuff())) { other_stuff(); }

things get ugly. We must similarly merge the conditional noexcept expressions. Naively, this looks right:

template <class T>
  requires A<T>
void f(const T&) noexcept(B<T> ? noexcept(other_stuff()) : noexcept(stuff())) {
  if constexpr (B<T>) {
    other_stuff();
  } else {
    stuff();
  }
}

but it's frequently the case that other_stuff is ill-formed when !B<T> and this won't work. An easy workaround is to create a separate bool variable template to calculate the conditional noexcept, mapping the if constexpr conditions to partial specializations, but things can quickly get out of hand in a more complicated real-world case. (This is the place where I find myself wishing for noexcept(auto).)

There were a great many occurrences of overload sets like this in the Ranges TS wording since the TS was based on C++14 which predates the introduction of if constexpr. I made at least one pass through the wording performing this transformation during the work to merge the TS with the IS - it helps implementers when we specify things how we intend them to implement - but there are still a few cases that slipped into the working draft.

[MahmoudGSaleh]

Looks good.

[MikeGitb]

but it's frequently the case that other_stuff is ill-formed when !B and this won't work. An easy workaround is to create a separate bool variable template to calculate the conditional noexcept, mapping the if constexpr conditions to partial specializations, but things can quickly get out of hand in a more complicated real-world case. (This is the place where I find myself wishing for noexcept(auto).)

Not sure, if this is relevant/helpful, but I don't think you need partial template specializations. You can just write

template<class T>
constexpr bool _F_is_noexcept = [](){
    if constexpr(B<T>){
        return noexcept( other_stuff<T>() ); 
    } else {
        return noexcept( stuff<T>() );
    }
}();

I think you could even write:

template <class T>
requires A<T>
void f3(const T&) noexcept([](){
        if constexpr(B<T>){
            return noexcept( other_stuff<T>() ); 
        } else {
            return noexcept( stuff<T>() );
        }
    }()) 
{
    if constexpr (B<T>) {
        other_stuff<T>();
    } else {
        stuff<T>();
    }
}

And avoid the separate variable altogether, but I'm not sure,if that is something desirable.

[CaseyCarter]

Not sure, if this is relevant/helpful, but I don't think you need partial template specializations. You can just write

template<class T>
constexpr bool _F_is_noexcept = [](){
    if constexpr(B<T>){
        return noexcept( other_stuff<T>() ); 
    } else {
        return noexcept( stuff<T>() );
    }
}();

The equivalent partially-specialized variable template is both slightly more terse:

template<class T>
constexpr bool _F_is_noexcept = noexcept(stuff<T>());
template<B T>
constexpr bool _F_is_noexcept<T> = noexcept(other_stuff<T>());

and slightly better for compiler throughput since it avoids using the constexpr interpreter.

I think you could even write:

template <class T>
requires A<T>
void f3(const T&) noexcept([](){
        if constexpr(B<T>){
            return noexcept( other_stuff<T>() ); 
        } else {
            return noexcept( stuff<T>() );
        }
    }()) 
{
    if constexpr (B<T>) {
        other_stuff<T>();
    } else {
        stuff<T>();
    }
}

And avoid the separate variable altogether, but I'm not sure, if that is something desirable.

This was one of the stops I made on my trip to developing the style used for customization point objects in the STL, which have much more complicated decision trees. There are effectively three different points in a function template that need to express roughly the same decision tree: (1) the constraints on the template itself when determining which arguments to accept, (2) the dispatch mechanism in the actual function body, and (3) the conditional noexcept determination. Our CPOs stuff the decision tree into a separate consteval function which effectively returns a (strategy, is_noexcept) tuple so the facade function can be simplified to something like:

template <class.. Args, auto Determination = decision_tree<Args...>()>
  requires Determination.strategy != None
decltype(auto) f(/* ... */) noexcept(Determination.is_noexcept) {
  if constexpr (Determination.strategy == meow) {
    // ...
  } else if constexpr (Determination.strategy == woof) {
    // ...
  }
}

switch constexpr (Determination.strategy) would make this even cleaner, but I'm very pleased that we've eliminated the repetition of the decision tree completely, even if it we must engage the constexpr interpreter to do so.

(Thanks for coming to my TED talk!)