12

I have the following code

auto adder = [](string& s1, const string& s2)->string&&
   {
      if (!s1.empty())
         s1 += " ";
      s1 += s2;
      return move(s1);
   };

   string test;
   test.reserve(wordArray.size() * 10);
   string words = accumulate(wordArray.begin(), wordArray.end(), 
       move(test), adder);

What I would like here is to avoid string copying. Unfortunately this is not accomplished by the vs2012 implementation of accumulate. Internally accumulate calls another function _Accumulate and the rvalue functionality gets lost in the process.

It I instead call the _Accumulate function like so

string words = _Accumulate(wordArray.begin(), wordArray.end(), 
    move(test), adder);

I get the intended performance gain.

Must the std library be rewritten to take rvalue arguments into consideration?

Is there some other way I may use accumulate to accomplish what I want without cheating too much?

Ralph Tandetzky
  • 22,780
  • 11
  • 73
  • 120
  • 1
    your `test` is not an rvalue. the `s1` in lambda is not an rvalue – BЈовић Dec 05 '12 at 14:48
  • C++20 specifies the operation as `acc = move(acc) + rhs`, which can considerably cheapen accumulation for types that aren't cheap to copy. For example, a good `std::string` implementation will have an `operator+(string&& lhs, T)` that hijacks the `lhs`, appends to that, and returns it (which is RVOable). – underscore_d Sep 24 '18 at 16:28

1 Answers1

4

Checking one of the recent post C++11 drafts (N3337.pdf) we can see that the effect of std::accumulate is specified as

Computes its result by initializing the accumulator acc with the initial value init and then modifies it with acc = acc + *i or acc = binary_op(acc, *i) for every iterator i in the range [first,last) in order.

So, the standard actually forbids implementations that use std::move for the old accumulator value like this:

template <class InputIterator, class T, class BinOp>
T accumulate (InputIterator first, InputIterator last, T init, BinOp binop)
{
  while (first!=last) {
    init = binop(std::move(init), *first);
    ++first;
  }
  return init;
}

which is unfortunate in your case.

Option (1): Implement this move-aware accumulate yourself.

Option (2): Keep using a functor like

struct mutating_string_adder {
  string operator()(string const& a, string const& b) const {return a+b;}
  string operator()(string & a, string const& b)      const {a += b; return std::move(a);}
  string operator()(string && a, string const& b)     const {a += b; return std::move(a);}
};

Note that I did not use rvalue reference return types here. This is intentional since it might avoid dangling reference issues, for example in the case where the last overload is picked and 'a' initialized to refer to a temporary object. All the operator+ overloads for strings also intentionally return by value.

Apart from that you might want to use std::copy in combination with std::stringstream and an output stream iterator.

Addendum: Alternate mutating_string_adder with some partial perfect forwarding:

struct mutating_string_adder {
  template<class T, class U>
  std::string operator()(T && a, U && b) const {
    return std::move(a) + std::forward<U>(b);
  }
};
sellibitze
  • 27,611
  • 3
  • 75
  • 95
  • `mutating_string_adder` - the 2nd operator(), a is lvalue, therefore move doesn't do anything. However, I am not sure if the 1st operator() should use move. – BЈовић Dec 06 '12 at 10:15
  • @BЈовић: The purpose of std::move is to turn an lvalue into an rvalue. So, sure, std::move does something. If I removed std::move around a in the return statement, the return value would be copy constructed instead of move constructed. There is no need to use std::move for the first overload because a+b already is an rvalue. – sellibitze Dec 06 '12 at 10:25
  • The move-aware accumulate worked fine with no copies anywhere and the same reserved area used all the way – Rolf W. Petersen Dec 06 '12 at 11:01
  • Right for the 1st, but I think you are wrong for the 2nd. Why would you turn lvalue into rvalue? Is that was possible, the object that is passed to s1 would be moved, and wouldn't exists anymore. Therefore, accessing it would be UB – BЈовић Dec 06 '12 at 11:49
  • @BЈовић: I already said why std::move is used in the 2nd operator() overload. Sorry, I don't see any problems. Where exactly do you think there is an access of a moved-from object -- other than the assignment in the move-aware version of accumulate? – sellibitze Dec 06 '12 at 16:50
  • One might expect the ref ref returned by adder to be insignificant. It turns out however that adding the && on the return enables the compiler to optimize away the call to the lambda function entirely. All that is left in the innermost loop is the string append and the iterator increment and comparison. – Rolf W. Petersen Dec 07 '12 at 12:42
  • C++20 specifies the operation as `acc = move(acc) + rhs`. – underscore_d Sep 24 '18 at 16:23