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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
