aaron.ballman added a comment. In D83717#2364187 <https://reviews.llvm.org/D83717#2364187>, @gamesh411 wrote:
> In D83717#2279263 <https://reviews.llvm.org/D83717#2279263>, @aaron.ballman > wrote: > >> One of the concerns I have with this not being a cfg-only check is that most >> of the bad situations are not going to be caught by the clang-tidy version >> of the check. > > ... > >> Have you run this check over any large code bases to see if it currently >> catches any true positive diagnostics? > > I have tried llvm, tmux, curl and tried codesearch.com to look for other > sources containing `atexit`, but no clang-tidy results were found for this > check (this is partly because it is hard to manually make the project results > buildable). So it is hard to see whether this flow-sensitive approach would > result in many false positives. Just to make sure we're on the same page -- the current approach is not flow-sensitive and so my concern is that it won't report any true positives (not that it will be prone to false positives). Btw, one trick I've used to make random projects easier to work with in clang-tidy is to either find ones that support CMake already or if they use make, then run the command through `bear` (https://github.com/rizsotto/Bear) -- this way I can get a compile_commands.json file that I can use to try to get clang-tidy diagnostics out of the project. In any of the projects that you found which are using `atexit()`, did you try to inspect the exit handler code paths to see if you could manually identify any problem code (like `assert()` calls)? I realize that's a lot of effort to go through (especially if you have to consider C++ constructs like constructors or overloaded operators which may be hard to spot by code inspection), but if you find the check doesn't trigger on code bases but there are issues when manually inspecting the code, that strongly suggests this should be a flow-sensitive check that probably lives in the static analyzer rather than clang-tidy. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83717/new/ https://reviews.llvm.org/D83717 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
