https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104251
Jonathan Wakely <redi at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |RESOLVED
Resolution|--- |INVALID
--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Felix Köhler from comment #0)
> a) the arguments with a non-default BinOp parameter seem to get scrambled,
> see attachment test01().
>
> vector<int> x={1,2,3,1};
> int sum;
> auto b=x.cbegin();
> auto e=x.cend();
> cout << reduce(b, e, 0, [](int sum, int i) {return sum+i+i;}) << "\t";
>
> yields 14 for CLANG, 30 for GCC. Trace of the lambda function calls included
> in the attachment.
Both results are allowed by the standard.
> b) Even a simple double addition lambda function does not work, if the
> accumulator value requires a higher precision to hold the result, than can
> be stored in the type of the sequence to be reduced. See attachment
> test02(). This is probably not the intended behavior.
>
> vector<unsigned char>bytes = {200, 201, 202, 203};
> cout << reduce(bytes.begin(), bytes.end(), 0L,
> [](long sum, unsigned char i) {return sum+i;});
>
> yields the expected 806 for CLANG, GCC overflows and yields 38.
Technically, not an overflow, because unsigned types don't overflow. It's a
reduction modulo 255.
Both results are allowed by the standard.
> c) Using non-implicitely-convertible types for the accumulator and the
> reduced function is not supported in GCC, yet CLANG compiles and runs the
> following code just fine. See test03().
>
> TLDR:
> vector<string> numbers={"eins","dos", "three", "quatre"};
> cout << reduce(numbers.begin(), numbers.end(), 0L,
> [](size_t sum, string s) {return sum+s.size();}) << "\t";
>
> CLANG counts 18 letters, GCC refuses compilation with an error message that
> the init value 0L cannot be converted into the sequence type of string.
GCC is correct. Clang is wrong.
> According to cppreference.com:
> "The behavior is non-deterministic if binary_op is not associative or not
> commutative. ... in other words, reduce behaves like std::accumulate except
> the elements of the range may be grouped and rearranged in arbitrary order."
> which seems to support the CLANG interpretation.
Eh? No it doesn't.
The standard is very clear:
Mandates: All of
— binary_op(init, *first),
— binary_op(*first, init),
— binary_op(init, init), and
— binary_op(*first, *first)
are convertible to T.
For your [](size_t, string) function that is clearly not met, and so the
implementation is *required* to give a diagnostic. If clang's libc++ compilers
it happily, that's a conformance bug in libc++.
> I'm not a language lawyer, GCC might be correct in assuming that the
> sequence MUST have the same type as the result of the reduce function and
GCC doesn't assume that though.
It's fine to use [](long i, long j) { return i + j; } to sum vector<int>, for
example.
It's not fine to use [](char i, char j) { return i+j; } if the total won't fit
in a char, and since it's unspecified which order the arguments are passed to
the function, your examples are broken too.
> you cannot expect to get passed the accumulator value as the first argument
> to BinOp and the sequence value as the second argument.
This is correct, you cannot assume that.
> However, this
> reduces std::reduce to a trivial SUM/PRODUCT function with a horrible
> interface and really evil implicit type-casting traps.
Only if you do silly things like try to use char to accumulate values that
overflow char.
> This seems to be a bug in either the libstdc++ implementation or the
> standard itself, please correct me, if I'm wrong.
You are wrong.
As Andrew said, the point is that the elements of the range can be passed to
the binary function in any order, which means you can process more than one
element per iteration of the loop.
If you want to process each element in order and have guarantees about the
arguments passed to the function, use std::accumulate.
If your function is associative and commutative, and you don't need the
guarantees or std::accumulate, you can use std::reduce. Otherwise, don't.
