Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[res.on.arguments.1.3] Can std library APIs that take arguments by rvalue reference move from them? #4377

Closed
gonzalobg opened this issue Nov 16, 2020 · 6 comments

Comments

@gonzalobg
Copy link

gonzalobg commented Nov 16, 2020

We have APIs like std::barrier::arrive that produce a std::barrier::arrival_token, that ought to be consumed at most once, e.g., by std::barrier::wait(arrival_token&&):

auto token = barrier.arrive();
barrier.wait(move(token));
barrier.wait(move(token)); // BUG

For this code to actually contain a bug, the user should not be able to rely on barrier::wait(move(token)) not executing the move constructor or move assignment operator of arrival_token.

The Effects clause of barrier::wait http://eel.is/c++draft/thread.barrier.class#20 does not have executing the move assignment operator or move constructor of arrival_token as an Effect.

The wording of [res.on.arguments.1.3] (https://github.com/cplusplus/draft/blob/master/source/lib-intro.tex#L2722-L2739, http://eel.is/c++draft/library#res.on.arguments-1.3) states:

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 1: If the parameter is a generic parameter of the form T&& and an lvalue of type A is bound, the argument binds to an lvalue reference ([temp.deduct.call]) and thus is not covered by the previous sentence. — end note]

[Note 2: 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 std​::​move(x)), the program is effectively asking that function to treat that lvalue as a temporary object. The implementation is free to optimize away aliasing checks which might be needed if the argument was an lvalue. — end note]

IIUC, the intent is to address use cases like the above.

I think this sentence's goal was to enable the standard library to move from rvalue arguments:

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 std​::​move(x)), the program is effectively asking that function to treat that lvalue as a temporary object.

But this sentence doesn't really allow an implementation to do that.

I think this is important enough to deserve a sentence outside of a note. Maybe:

If a function argument binds to an rvalue reference parameter, 
the implementation may assume that this parameter is a unique 
-reference to this argument.
+reference to this argument and move its value out.
@gonzalobg gonzalobg changed the title Can std library APIs that take arguments by rvalue reference move from them? [res.on.arguments.1.3] Can std library APIs that take arguments by rvalue reference move from them? Nov 16, 2020
@jwakely
Copy link
Member

jwakely commented Nov 16, 2020

This seems non editorial and should be sent to https://cplusplus.github.io/LWG/lwg-active.html#submit_issue

@jwakely
Copy link
Member

jwakely commented Nov 16, 2020

That said, if the rvalue reference is a unique reference to the argument, the caller can't tell if it gets moved. The intent of Note 2 is to say that explicitly casting an lvalue to an rvalue and passing it to a function that binds an rvalue reference to it can assume the caller isn't going to inspect it, or at least can't assume it won't be moved.

@gonzalobg
Copy link
Author

gonzalobg commented Nov 16, 2020

That said, if the rvalue reference is a unique reference to the argument, the caller can't tell if it gets moved.

I read those sentences, but couldn't answer the question: "unique reference, in which scope?". The Note 2 says:

The implementation is free to optimize away aliasing checks which might be needed if the argument was an lvalue.

I read this to mean that the reference can be assumed to be unique within the implementation of the standard library API, in pretty much the same way that C's restrict works.

That is:

foo a, b, c;
void std_lib(foo&& a, foo&& b); // similar to `std_lib(foo restrict* a, foo restrict* b);
std_lib(move(a), move(b)); // OK
std_lib(move(c), move(c)); // UB: violates unique reference

I'm not sure what else it could mean either. If we extend the scope to the caller, then it could mean that, e.g.,

auto token = barrier.arrive();
auto ptr = &token; // copy address
barrier.wait(move(token));  // UB: not unique

This wording about "unique reference" and its intent could definitely be made more clear as well.

@jwakely
Copy link
Member

jwakely commented Nov 17, 2020

auto token = barrier.arrive();
auto ptr = &token; // copy address
barrier.wait(move(token));  // UB: not unique

That's what it means, except it doesn't say anything about it being undefined to have another reference. It just means the library can assume that the caller cannot (or will not) inspect the value after the call. Therefore, it's OK to move from it. The origin of the wording is LWG issue 1204: Global permission to move. The name of the issue and the discussion make it pretty clear moving is intended. It says "If v is truly bound to a temporary, then push_back has the only reference to this temporary in the entire program." It's definitely not talking about "unique within the implementation of the standard library API", it's talking about the whole program.

This is clearly not editorial and so this is the wrong venue to discuss it.

@gonzalobg
Copy link
Author

Thanks, I'll file a proper issue. I agree that the intend is clear, which is why I thought that this clarification could be an editorial change. Sorry for the misunderstanding.

@jwakely
Copy link
Member

jwakely commented Dec 3, 2020

This is moving to the LWG issues list.

@jwakely jwakely closed this as completed Dec 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants