[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: |
|
||||||||||||
| 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 |
|
``` void ``` 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: No need to have two distinct ctors. Just create one and use forwarding. | ||
| 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 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:
`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. |