This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On 11 Jan 2011, at 12:43, Bo Persson 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!

It is my belief that indeed we can't be sure that x=move(x) will behave correctly.

However, doing x = f(move(x), y) should be fine. Unless of course f just returns an rvalue reference to it's first argument. But then f is a very badly behaved function, possibly with other issues.

Chris


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]