[PATCH] D54438: [analyzer][WIP] Reimplement dependencies between checkers

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 12 14:18:25 PST 2018


NoQ added a comment.

Hmm.

All intentions in this patch are great and i love them!

I think that the refactoring work in `MallocChecker` is a bit pre-mature; i don't have a clear picture of how the ideal `MallocChecker` should look like, and i'm not sure this patch moves us into the right direction. Like, it tries to deal with multiple API packs (malloc-free, new-delete, new[]-delete[], g_malloc()-g_free(), alloca()-???, obtaininnerpointer-invalidateinnerpointer, `CStringChecker` interop) and multiple warning kinds (use-after-free, double free, leak, free at offset, mismatched deallocators) and i've no idea what's the proper way to split this into smaller checkers, how many `Checker` sub-classes do we need, how should they be split up into multiple files and inter-operate via headers (maybe it's better for sub-checkers include `MallocChecker.h` than for `MallocChecker` to include sub-checkers' `.h`s?), what exactly should the "base" checker do, how many `Checkers.td` entries do we need, how should the `Checkers.td` dependency graph look like, AAAAAAAAAAAAAA. I want to see a picture of all this stuff drawn first, before jumping into solving this.

Can we reduce this patch to simply introducing the dependency item in `Checkers.td` and using it in, like, one place (eg., `MallocChecker`/`CStringChecker` inter-op) and discuss the rest of the stuff later before we jump into it?


Repository:
  rC Clang

https://reviews.llvm.org/D54438





More information about the cfe-commits mailing list