Use explicit captures in lambda expressions #13770

1. 
   practicalswift commented at 4:01 pm on July 26, 2018: contributor

   Use explicit captures in lambda expressions.

   Rationale:

   • Explicit is better than implicit.
   • Help avoid the lambda dangling reference problem (UB) by being explicit with what objects we use. Makes it easier to reason about their lifetimes.
2. 
   DrahtBot commented at 4:55 pm on July 26, 2018: member
3. DrahtBot added the label Needs rebase on Jul 26, 2018
4. MarcoFalke added the label Refactoring on Jul 26, 2018
5. practicalswift force-pushed on Jul 26, 2018
6. 
   practicalswift commented at 8:47 pm on July 26, 2018: contributor

   Rebased!
7. DrahtBot removed the label Needs rebase on Jul 26, 2018
8. 
   promag commented at 2:05 pm on July 27, 2018: member

   I’m -0 on this. Even if this is merged, how could we prevent implicit captures in the future?
9. 
   practicalswift commented at 4:48 pm on July 27, 2018: contributor

   @promag What are the best arguments in favour of implicit captures? :-)
10. Use explicit captures in lambda expressions when capturing is non-trivial (more than one capture) a4def4dd54
11. 
    laanwj commented at 12:26 pm on July 30, 2018: member

    Tend towards NACK. Sorry to rant, but I’m starting to feel quite strongly about this:

    If you want to make changes like this, the way to do so is:

    • create a PR to update the coding guidelines with your motivation on how this avoids bugs (as you did in the OP), and that your point is strong enough for this to be merged
    • if it is merged, make sure it is applied to new code, in reviews
    • then maybe, in the longer term, if people agree that this is important, do a PR that updates old occurrences

    Please don’t create a PR out of the blue that changes some formerly unheard-of thing in the entire codebase. This creates review work, extra risk, and all massive idempotent code changes all over the code make it harder to keep track of what is actually changing.

12. practicalswift force-pushed on Jul 30, 2018
13. 
    practicalswift commented at 12:34 pm on July 30, 2018: contributor

    @promag Thanks for reviewing. I’ve now updated the PR. Implicit captures are now kept for trivial lambdas with only one capture. Explicit captures are used when the capture count is two or above. What do you think? Please re-review :-)
14. 
    practicalswift commented at 2:16 pm on July 30, 2018: contributor

    @laanwj Makes sense! Closing this PR
15. practicalswift closed this on Jul 30, 2018

    ---

16. practicalswift deleted the branch on Apr 10, 2021
17. DrahtBot locked this on Aug 16, 2022

Contributors
practicalswift DrahtBot promag laanwj

Labels
Refactoring