Parallel for_each iterators don't have to be mutable

[dkolsen-pgi]

The overloads of for_each and for_each_n that take an execution policy argument do not require that the iterator arguments always be mutable iterators. Whether or not the iterators need to be mutable depends on what the function object does. for_each and for_each_n are unusual, if not unique, in this regard; for all other algorithms the mutability of the iterators is specified by the algorithm. See http://eel.is/c++draft/alg.foreach#7 , though I admit that the wording is not particularly clear about this.

Since the compiler can't reliably figure out whether or not the function object modifies the elements in the sequence, for_each and for_each_n should only check for constant iterators (_REQUIRE_PARALLEL_ITERATOR), not mutable iterators (_REQUIRE_CPP17_MUTABLE_ITERATOR).

This bug was introduced by #2960 . That PR should not have changed for_each or for_each_n.

[dkolsen-pgi]

This is an important fix. My experience with C++ parallel algorithms is that it is very common to use for_each with a counting iterator of some sort, especially when porting existing code that uses for loops. Until now, people have used a hand-crafted counting iterator or counting_iterator from Thrust or Boost, where the iterator could claim to be random access even though it was technically not. As C++20 and C++23 become available, people will switch to using views::iota and views::cartesian_product instead of a non-standard counting iterator. The iterators from those views need to work with parallel for_each, and they will be rejected without this change.

For example:

#include <iostream>
#include <algorithm>
#include <execution>
#include <ranges>

int main()
{
    auto numbers = std::views::iota(0, 100);
    std::cout << "Hello World!\n";
    std::for_each(std::execution::par, numbers.begin(), numbers.end(), [](int i) { std::cout << i << " "; });
    std::cout << "\n";
}

That test program requires this change to compile. (The output will be garbled and in a sort of random order due to the parallelism, but I was just looking for a very quick test case.)

[StephanTLavavej]

Thanks for this fix! (And noticing/fixing it so quickly, before STL changes stop flowing into VS 2022 17.4 Preview 3 - we should be able to prevent the bug from ever shipping to customers.)

I remember thinking about this question while reviewing the PR (especially because LWG-475 "May the function object passed to for_each modify the elements of the iterated sequence?" was my first submitted issue in 2004 😹), but I didn't think through all the consequences. Your example is compelling.

[dkolsen-pgi]

Credit for finding the bug goes to Pili Latiesa. They contacted me as the author of P2408 to ask if views::zip iterators should be usable with for_each and pointed to the _REQUIRE_CPP17_MUTABLE_ITERATOR macro in for_each that would prevent that.

[StephanTLavavej]

I'm speculatively mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

[CaseyCarter]

LGTM. Thanks for the fix, and thanks to Pili for finding this.

[strega-nil-ms]

Mmh, I'll note that this isn't a regression, however, this does fix a bug in implementing P2408R5. Thanks for noticing and the quick fix!

[StephanTLavavej]

Thanks again for fixing this, and congratulations on your first microsoft/STL commit! 😸 🎉 🚀