[PATCH] D45458: MallocChecker refactoring of calls checkers

Gábor Horváth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 12 08:20:45 PDT 2018


xazax.hun added a comment.

In https://reviews.llvm.org/D45458#1062342, @NoQ wrote:

> @xazax.hun: do you think the approach you took in the `Valist` checker is applicable here? Did you like how it ended up working? Cause i'd love to see `CallDescription` and initializer lists used for readability here. And i'd love to see branches replaced with map lookups. And i'm not sure if anybody has written code that has all the stuff that i'd love to see.


Overall the `CallDescription` interface works really well if you do not need anything more advanced, e.g. supporting overloads. I can imagine that its current implementation is slightly less efficient since the lazy initialization logic of the identifiers are repeated for every identifier but that also makes it less bug-prone (do not have the implicit assumption that a set of identifiers should always exist together).

It would be great to extend the interface in the future, but one of the reasons I was reluctant to do so yet is performance. E.g.: having a simple check that a checker is interested only in global C functions is better than having this check repeated in every call description object in an array, or making it possible to do a StringSwitch like optimization with multiple statically known CallDescription objects would also be awesome.

But I think most of the time these tests would not be used on the hot paths anyways, so as long its current expressive power is sufficient, I prefer code that is utilizing this tool. It tends to make the code slightly shorter.


Repository:
  rC Clang

https://reviews.llvm.org/D45458





More information about the cfe-commits mailing list