[PATCH] D96215: [clang-tidy] Recognize captures as a form of aliasing.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: alexfh, gribozavr2, aaron.ballman, vsavchenko.
Herald added subscribers: nullptr.cpp, martong, mgehre, Charusso, xazax.hun.
NoQ requested review of this revision.

The utility function `clang::tidy::utils::`()` scans the function for pointer/reference aliases to a given variable. It currently scans for operator `&` over that variable and for declarations of references to that variable.

I'm arguing that it should scan for lambda captures by reference as well. If a lambda captures a variable by reference it basically has a reference to that variable that the user of the lambda can access at any time, including on background thread, which may have meaningful impact on checkers. The same applies to blocks (an Apple extension that brings closures to C and Objective-C; blocks are also implicitly convertible with lambdas when both facilities are available).

The patch fixes false positives in both checkers that use this functionality: `bugprone-infinite-loop` and `bugprone-redundant-branch-condition`. There are a few false negatives it introduces as a tradeoff: if the lambda captures a variable by reference but never actually mutates it we'll no longer warn but we should (as demonstrated by FIXME tests). These false negatives are architecturally annoying to deal with because `hasPtrOrReferenceInFunc()` detects aliasing rather than mutation, so it'll have to be a different facility entirely. They're also rudimentary because there's rarely a good reason to write such code; it probably deserves a separate warning to relax capture kind to a by-value or by-const-reference capture (which is often explicit). That said, C++'s `[&]` would probably capture a lot of stuff by reference unnecessarily and it'll often make their code worse if developers are forced to write down all captures manually instead.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D96215

Files:
  clang-tools-extra/clang-tidy/utils/Aliasing.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D96215.321973.patch
Type: text/x-patch
Size: 5814 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210207/5948c03b/attachment.bin>