[clang] [clang-tools-extra] RFC: [clang-tidy] [analyzer] Move nondeterministic pointer usage check to tidy (PR #110471)

[Julian Schmidt via cfe-commits]

5chmidti wrote:

> I'd like to keep the scope limited for this PR to just moving the static analysis checks to a single clang-tidy check. Would that be ok with the reviewers?

Fine with me, yes, but there needs to be some sort of plan on how to address some of the pointed out challenges with this check, that were raised by others.

---

> It might be possible, but really really challenging to implement.
> ...
> Only looking at the for loop here is not enough ...

+1

> because we apply a sideffect that is dependent on the nondeterministic iteration order

> What if we tried to match archetypes of "container elements are printed" or "container elements are sent to the network"? It will never be a complete list, but we could reasonably assume (at least this is a safe-ish generalisation, and for everything else, there is NOLINT...) that if the order escapes via some I/O, then non-determinism is... maybe if not outright bad, but definitely uncanny, if for nothing else, due to the troubles it causes when debugging.

That's of course possible, but may grow to 'too many' patterns fast, and then there comes the question of which ones to add and which are considered to be too infrequent. The other issue is that doing this would mean that either clang-tidy has to encode the use of many different libraries, and how side effects can happen by using them, or by providing customization options, which would in turn be limited by the patterns that are considered generic enough to warrant the customizable detection pattern (e.g., printing and formatting, and then there are `formatToString` member functions as well, that e.g., print into a preallocated buffer).

Maybe `Expr::hasSideEffets` can be used as a baseline?
I.e.,
```
for all references r to var of type unordered map
  traverse up the AST until the top-level expression e (not stmt) is found (containing r)
    if e->hasSideEffects()
      diagnose()
```
(maybe not literally, to avoid using the parent map too much for traversal)

For calling functions, there could also be a (cached) lookup of their bodies (if available), using `Expr::hasSideEffects` for the `DeclRefExpr` of the parameter. `bugprone-exception-escape` is doing something similar, but for `throw` statements, instead of checking for variable uses with side effects.

https://github.com/llvm/llvm-project/pull/110471