[PATCH] D68162: [analyzer][MallocChecker][NFC] Communicate the allocation family to auxiliary functions with parameters

Kristóf Umann via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 11 07:27:23 PDT 2019


Szelethus added a comment.

I forgot to emphasise that the entire point of the patch was to get rid of `getAllocationFamily`, at least the way it used to work, because it was a mess and prevented me from moving forward with changing things to a `CallDescriptionMap`.

In D68162#1693110 <https://reviews.llvm.org/D68162#1693110>, @baloghadamsoftware wrote:

> I like this change. If we can retrieve something with a simple getter, then do not carry extra parameters all over the code. However, if we have to recalculate something over and over again then it is much better to determine it once and pass it around as a parameter. Especially in this case where we have the information statically at top level and do not have to calculate it all if we use that.




In D68162#1686810 <https://reviews.llvm.org/D68162#1686810>, @NoQ wrote:

> My mental model for this sort of stuff is something like this:
>
>   CallDescriptionMap M = {
>     "malloc", {&MallocChecker::MallocMemAux, AF_Malloc},
>     "alloca", {&MallocChecker::MallocMemAux, AF_Alloca},
>   };
>  
>   void checkPostCall(const CallEvent &Call, CheckerContext &C) {
>     if (Info *I = M.lookup(Call))
>       I->first(I->second, C, Call);
>   }
>


I guess you two are right. Maybe the best solution would be collapse `const CallExpr *` and `AllocationFamily` parameters into `CallEvent`, and just query the relevant information. Sure, its a query every time we need it, but nothing impacts performance as much as months of hair pulling trying to understand what this file does :^)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68162/new/

https://reviews.llvm.org/D68162





More information about the cfe-commits mailing list