[PATCH] D68165: [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy CallDescriptionMap

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.

Looks wonderful, thanks!



================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:355
+  template <bool ShouldFreeOnFail>
+  void checkRealloc(CheckerContext &C, const CallExpr *CE,
+                    ProgramStateRef State) const;
----------------
Charusso wrote:
> balazske wrote:
> > The `CHECK_FN` could be used even here?
> > ```
> > template <bool ShouldFreeOnFail>
> > CHECK_FN(checkRealloc)
> > ```
> I think about the opposite, passing around arguments by templates is not cool. They meant to be arguments.
> 
> This type of callback simply could be a third `CallDescriptionMap` for future profeness.
> passing around arguments by templates is not cool. They meant to be arguments.

+1! It's accidental that these values are currently known at compile time.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1184
+    checkCXXNewOrCXXDelete(C, CE, State);
+
+  checkOwnershipAttr(C, CE, State);
----------------
balazske wrote:
> If these cases are exclusive the code looks better when `else` statements are added?
Yup, or even better, `return`s, because this code is very fragile because it'll do horrible things if we accidentally invoke two callbacks at once.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68165