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

[Bo Persson]

Johannes Schaub wrote:

>> 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.
>

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