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

[Johannes Schaub (litb)]

> Johannes Schaub wrote:
> 
> > Bo Persson <bop <at> gmb.dk> writes:
> > 
> >> 
> >> 
> >> On 10 jan 2011 21:15 "Jonathan Wakely" <jwakely.gcc <at> gmail.com>
> >> wrote:
> >> > On 10 January 2011 18:47, Bo Persson <bop <at> 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!
> >> 
> > 
> > [The following is a repost of a message I wrongly sent before as a
> > [non-follow-
> > up. I'm reposting it as a follow-up so this won't end up in a mess. I 
hope
> > this won't offend anyone.]
> > 
> > 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.
> 
> Maybe. :-)
> 
> However, the MoveAssignable requirement is stated for the expression t 
> = rv, not for t = move(t). Does that make a difference? It might.
> 
> 
> Bo Persson
> 

See 20.2.1/1 for what "rv" stands for. There's no way around this: The 
requirement clearly says that "t = move(t)" shall result in a no-op wrt the 
value of t. I think the only question is whether 17.6.3.9/1 can overrule it 
for classes defined in the standard library, and I don't know the answer.