[PATCH] Exploit move semantics in std::accumulate.

[Johannes Schaub]

> On 10 jan 2011 21:15 "Jonathan Wakely" <jwakely.gcc@gmail.com> wrote:
> > On 10 January 2011 18:47, Bo Persson <bop@gmb.dk> wrote:
> > > Joe Buck wrote:
> > >> On Sat, Jan 08, 2011 at 01:44:21PM -0800, Eelis van der Weegen
> > >>
> > >> wrote:
> > >>> I briefly talked to a committee member who mentioned something
> > >>> about Chris already having brought the issue up on the
> > >>> "non-public-yet-not-secret" mailing list, which would remove the
> > >>> need for me to do so on the public mailing lists you mention. I
> > >>> merely asked for confirmation.
> > >>
> > >> I hope that the committee will look at the issue not just in the
> > >> context of this one algorithm, but will make sure that move
> > >> semantics can be legitimately exploited in all relevant
> > >> algorithms.
> > >
> > > I believe the idea is that as rvalue references are supposed to be a
> > > speed
> > > optimization, the intent of the current draft is to NOT require
> > > checks for
> > > self assignment from an rvalue.
> > >
> > > Section 17.6.3.9 [res.on.arguments] specifically says this about
> > > library
> > > functions:
> > >
> > > "If a function argument binds to an rvalue reference parameter, the
> > > implementation may assume that
> > > this parameter is a unique reference to this argument. ... [ Note:
> > > If a
> > > program casts an lvalue to
> > > an xvalue while passing that lvalue to a library function (e.g. by
> > > calling
> > > the function with the
> > > argument move(x)), the program is effectively asking that function
> > > to treat
> > > that lvalue as a temporary.
> > > The implementation is free to optimize away aliasing checks which
> > > might be
> > > needed if the argument
> > > was an lvalue. -end note ]"
> >
> > std::accumulate doesn't take it's init argument by reference, so that
> > doesn't apply.
> >
> > The Effects clause explicitly says the accumulation is done by "acc =
> > acc + *i" which is detectable by users
> > e.g
> > struct X { int i; };
> > X operator+(const X& x, int i) { return { x.i + i }; }
> > X operator+(X&& x, int i) = delete;
>
> Ok, I was thinking about the assignment operator, which actually gets a
> temporary here (from the addition). You show that there is still a
> problem.
>
> However, earlier in the thread someone said that move assignment is
> required to "do the right thing". The wording in the current draft though
> indicates that self-move-assignment is NOT required to work. This could
> affect x = move(y) in some algorithms, if x and y could possibly alias
> each other. We should be careful here!

I don't think the text you quoted from 17.6.3.9/1 grants classes to
ignore self-move-assignment: The text also says "Each of the following
applies to all arguments to functions defined in the C++ standard
library". Move assignment operators of classes not defined in the C++
standard libraries are therefor not caught. Therefor, I very much
doubt that 17.6.3.9/1 has anything to do with allowing to disregard
self-move-assignment.

Also, I'm not sure whether "unless stated otherwise, ... the
implementation may assume" has any effect on the MoveAssignable
requirement. The requirement is stated, and I have a hard time imaging
that allowing latitude like that could invalidate parts of the
requirement.

Just my silly thoughts about that.