[SERVER-42313] Fix forwarding reference issue in `ScopeGuard` Created: 22/Jul/19  Updated: 29/Oct/23  Resolved: 26/Jul/19

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 4.3.1

Type: Bug Priority: Major - P3
Reporter: ADAM Martin (Inactive) Assignee: ADAM Martin (Inactive)
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Backports
Gantt Dependency
has to be done before SERVER-41989 BSONObjBuilder::asTempObj is not exce... Closed
Backwards Compatibility: Fully Compatible
Operating System: ALL
Backport Requested:
v4.2, v4.0, v3.6
Sprint: Dev Tools 2019-07-15, Dev Tools 2019-07-29
Participants:

 Description   

```
template< typename T >

void
f( T &&t )

{   T x= std::move( t ); }

```

This is somewhat incorrect (even though this has a `const T &` overload in `ScopeGuard` – `std::forward< T >` is what should be used.

We should also consider removing the move operations entirely from `ScopeGuard` in favour of using the pure MCE/NRV/RVO transform.



 Comments   
Comment by Githook User [ 26/Jul/19 ]

Author:

{'name': 'ADAM David Alan Martin', 'username': 'adamlsd', 'email': 'adam.martin@10gen.com'}

Message: SERVER-42313 Fix forwarding in `ScopeGuard` ctor.

No need to have two distinct ctors. Just create one and use forwarding.
Branch: master
https://github.com/mongodb/mongo/commit/c44cf6f5f299991ec6819b5933b081476dc65a27

Comment by Mira Carey [ 22/Jul/19 ]

I'm strongly in favor of dumping the move opts. We can't stop RVO (so you have to think about it regardless), might as well let that be the way you factory construct a scope guard as well

Comment by ADAM Martin (Inactive) [ 22/Jul/19 ]

Indeed, there's no code using `F &&` as a forwarding reference, but this means that the conversion happens in the `makeScopeGuard` operation, rather than making it in the ctor itself.  It basically eliminates a ctor, the way I've done it.

MCE - Mandatory Copy Elision
NRV - Named Return Value Transform
RVO - Return Value Optimization

These three appear overlap or be the same, but do different, interlinking things.

MCE is when the standard says that a copy or move can be entirely eliminated, due to seeing how the object flows through the code.  Often allows direct construction into a paramater slot, or from a return slot, bypassing any need for a copy or move ctor.

NRV is the transformation the standard allows when all common paths through a function after the declaration of a named variable wind up returning that same named variable.  The Named Return Value can be constructed in the return-slot for the function.

RVO is the optimization path whereby the return slot of a function can be pointed at the memory where you want the returned object to be stored.

They cooperate to give the best return performance.

Comment by Billy Donahue [ 22/Jul/19 ]

Description needs to be clearer. What's "the pure MCE/NRV/RVO transform" ?

There's also no code in ScopeGuard that matches the opening snippet:

template< typename T >
void f( T &&t ) { T x= std::move( t ); }

`std::forward` is for a forwarding reference. The `ScopeGuard<F>` constructor's use of `F&&` isn't a deduced template parameter type of the constructor, it's a parameter of the class. The constructor is not a template.

Generated at Thu Feb 08 05:00:09 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.