[SERVER-54063] AsyncTry-until condition should have to take its input by const reference Created: 26/Jan/21 Updated: 29/Oct/23 Resolved: 02/Aug/23 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Internal Code |
| Affects Version/s: | None |
| Fix Version/s: | 7.1.0-rc0 |
| Type: | Improvement | Priority: | Minor - P4 |
| Reporter: | Matthew Saltz (Inactive) | Assignee: | Amirsaman Memaripour |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | neweng, re-triaged-ticket, servicearch-wfbf-day | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Assigned Teams: |
Service Arch
|
| Backwards Compatibility: | Fully Compatible |
| Sprint: | Service Arch 2023-07-10, Service Arch 2023-07-24, Service Arch 2023-08-07 |
| Participants: |
| Description |
|
I was helping diagnose a confusing situation yesterday where someone had something like this:
It fails to compile because the lambda passed to 'until' is called with an lvalue, so it tries to copy the move-only type and fails to. Furthermore, it has to pass it as an lvalue because it still needs to use it later, so moving it into the condition isn't an option. These compiler errors are kind of cryptic. If we added a static assertion to ensure that the condition passed to 'until' had to take a const ref, then we could avoid this situation by emitting an easy to diagnose compiler error. |
| Comments |
| Comment by Githook User [ 03/Aug/23 ] |
|
Author: {'name': 'Amirsaman Memaripour', 'email': 'amirsaman.memaripour@mongodb.com', 'username': 'samanca'}Message: |
| Comment by Amirsaman Memaripour [ 02/Aug/23 ] |
|
Decided to update the documentation and add a test that showcases the usage with move-only types. |
| Comment by Githook User [ 02/Aug/23 ] |
|
Author: {'name': 'Amirsaman Memaripour', 'email': 'amirsaman.memaripour@mongodb.com', 'username': 'samanca'}Message: |
| Comment by Billy Donahue [ 11/Jul/23 ] |
|
I don't think it's technically possible to restrict the argument in that way, to insist that it takes its argument by const ref and not by value. You can check that it CAN be invoked with a const ref, but you can't reliably tell whether its call operator takes its parameter by const ref. For example the Func::operator() for the callable could be overloaded or be a member function template (common with lambdas that take auto arguments), such that trying to test the type of that function will fail with ambiguity. It's likely that trying to be very restrictive here will cause false rejection of valid code, which would end up being at least as confusing as the error messages we're trying to avoid in the ticket description Those errors are really standard compiler errors and not specific to this function. I disagree with the description's statement that "These compiler errors are kind of cryptic". They are not provided here, but a deleted copy constructor is going to be the compiler's complaint, and the line number will be there. It's not an unusual message. I think we can close this ticket and move on to other things that aren't so tricky to specify. We could perhaps improve the documentation on the until function, and that would also help people who are struggling to use it correctly, and it's a cheap upgrade for us to make. It's currently entirely undocumented. It's unsurprising that there are struggles happening. The class-level docs for AsyncTry even give an example of a bad until function call, with a lambda that takes by value instead of by const ref. |
| Comment by Billy Donahue [ 06/Jul/23 ] |
|
|