Why does ranges::accumulate not pass init as std::move(init) when invoke?

Question

As of the commit d5e9afc on Mar 17,2018 of accumulate.hpp

When passing a range, the init gets std::move once like this.

        T operator()(Rng && rng, T init, Op op = Op{}, P proj = P{}) const
        {
            return (*this)(begin(rng), end(rng), std::move(init), std::move(op),
                std::move(proj));
        }

Above code will then call this:

        T operator()(I begin, S end, T init, Op op = Op{}, P proj = P{}) const
        {
            for(; begin != end; ++begin)
                init = invoke(op, init, invoke(proj, *begin)); // why do we need this another copy of init?
            return init;
        }

I wonder why do we need this another copy of init before call invoke?

This init must be overidden any way, right? So why is it not okey to rip it off in the first place?

                init = invoke(op, std::move(init), invoke(proj, *begin));

@StoryTeller Why would one prohibit moving things as parameters? — sandthorn, Apr 24 '18 at 11:14
My guess would be that `init` is intended to be a primitive type so passing by value could be more efficient. — Galik, Apr 24 '18 at 11:20
@Galik Moving a non-primitive taxes extra cycles? Or the library tries to encourage the use of `init` as a premitive? or both? — sandthorn, Apr 24 '18 at 11:26

StoryTeller - Unslander Monica · Answer 1 · 2018-04-24T12:46:29.467

3

This piece of code tries to avoid assuming C++17 it would seem. std::move-ing init could potentially modify it (that's what move semantics are for, at the end of the day). And that leaves us with something like this:

init = /* An expression that maybe modifies init */;

Which would result in undefined behavior prior to C++17. range-v3 advertises itself as a library for C++11 and C++14 as well.

edited Apr 24 '18 at 12:46

answered Apr 24 '18 at 11:13

StoryTeller - Unslander Monica

165,132
21
377
458

If this is only that, well do you think whether they will add C++17+ macro that move? – sandthorn Apr 24 '18 at 12:32
@sandthorn - IDK, I'm not in their head :) This is an open source project, send them a pull request if you see room for improvement. – StoryTeller - Unslander Monica Apr 24 '18 at 12:35
`undefined behavior prior to C++17` Would you mind pointing out which C++14 standard section that explicitly states the undefined? – sandthorn Apr 24 '18 at 12:40
1

@sandthorn - It's already mentioned on SO. See [here](https://stackoverflow.com/a/46171943/817643). – StoryTeller - Unslander Monica Apr 24 '18 at 12:42
After some reading, I found out that `std::accumulate` requires `CopyConstructible` from `init` so this must be left as is, until the day `std` changes by itself. Moreover, I guess range-v3 have a will to be complaint with ``. However, `std::reduce` now does require init `MoveConstructible`!!. Well, there seems no implementation out there yet, though. – sandthorn Apr 24 '18 at 13:32

Why does ranges::accumulate not pass init as std::move(init) when invoke?

1 Answers1