[SERVER-60173] Ban implicit capture of `this` via `[=]` in lambda expressions Created: 23/Sep/21 Updated: 29/Oct/23 Resolved: 23/May/23 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 7.1.0-rc0 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Andrew Morrow (Inactive) | Assignee: | Juan Gu |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | auto-reverted, post-c++20 | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||
| Assigned Teams: |
Server Development Platform
|
||||||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||||||
| Participants: | |||||||||||||||||||||
| Linked BF Score: | 157 | ||||||||||||||||||||
| Description |
|
The ask here is basically to remove `-Wno-deprecation` and fix the lambda problems that are a result of that. billy.donahue@mongodb.com 's comment is probably accurate about exactly what should be disabled. Quoting the same source this is what we are getting rid of.
|
| Comments |
| Comment by Githook User [ 22/May/23 ] |
|
Author: {'name': 'Juan Gu', 'email': 'juan.gu@mongodb.com', 'username': 'juangugit'}Message: |
| Comment by xgen-buildbaron-user [ 20/May/23 ] |
|
Ticket re-opened due to revert. compile_dist_test began a consistent failure of compile_dist_test |
| Comment by Githook User [ 20/May/23 ] |
|
Author: {'name': 'auto-revert-processor', 'email': 'dev-prod-dag@mongodb.com', 'username': ''}Message: Revert " This reverts commit 02bf3f55f88874bf397b42758c9cd36093633f9e. |
| Comment by Githook User [ 17/May/23 ] |
|
Author: {'name': 'Juan Gu', 'email': 'juan.gu@mongodb.com', 'username': 'juangugit'}Message: |
| Comment by Billy Donahue [ 25/Apr/23 ] |
|
Thanks alexander.neben@mongodb.com for the updated description. I went ahead and made the title equally specific. |
| Comment by Billy Donahue [ 24/Apr/23 ] |
|
The ticket is underspecified and has no description. I'm not entirely sure what it's proposing, but a reading at face value implies that it's proposing a ban of all [=] captures, which are implicit in that they rely on that default capture spec. But these are perfectly valid before and after C++20, and will be valid forever, presumably (per my previous comment). I think you're thinking of the implicit capture of *this (as a reference) when there's a default capture spec of [=]: https://en.cppreference.com/w/cpp/language/lambda#Lambda_capture
That's what we should be sweeping for, but this ticket doesn't say so explicitly and doesn't mention the *this reference at all. So maybe it needs an updated description with details on what it's talking about so that we don't end up trying to enforce a ban on good code by accident. |
| Comment by Alex Neben [ 14/Dec/22 ] |
|
I think the context here is that it is banned by default in c++20. This would be primitively fixing the problem. |
| Comment by Billy Donahue [ 23/Sep/21 ] |
|
-1: I think banning [=] is interfering with a valid programmer choice. I think we should be expected to apply good judgment in deciding which kind of capture to use. I don't use it often but I know where it's appropriate to lambda capture by-value and when it isn't. In a short lambda capturing a few variables and it all fits on a few lines, I don't see any risk. Retyping the variable names (especially if they are long names) just to bind them will obscure otherwise elegant and concise expressions. I would not want to uglify lambdas that are obvious and safe simply to satisfy a linter. It's also slightly more unfriendly to maintenance to have explicit captures. Instead of simply mentioning a variable in the lambda body, you have to mention it and add it to the capture list unnecessarily. Lambdas are meant to be usable as "1-liners", and [=] supports that notion. Removing it from the toolbox would be downgrade. I have heard the statement that "explicit is better than implicit" repeated a lot over the years, and it's not a true statement. |