[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

Some nits inline.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:995
 
-void MallocChecker::checkBasicAlloc(CheckerContext &C, const CallExpr *CE,
-                                    ProgramStateRef State) const {
-  State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State, AF_Malloc);
-  State = ProcessZeroAllocCheck(C, CE, 0, State);
+void MallocChecker::checkBasicAlloc(const CallEvent &Call,
+                                    CheckerContext &C) const {
----------------
What is the main reason of getting rid of the state parameter? I think this could make using these functions more error prone overall as it would be easier to introduce splits in the exploded graph this way (if we somehow wanted to model a function as successive calls of some other functions).


================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1241
     Arg = CE->getArg(IndexOfSizeArg);
-  }
-  else if (const CXXNewExpr *NE = dyn_cast<CXXNewExpr>(E)) {
-    if (NE->isArray())
+  } else if (const CXXNewExpr *NE =
+                 dyn_cast<CXXNewExpr>(Call.getOriginExpr())) {
----------------
Use `auto` here to avoid repeating the type.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1641
 ProgramStateRef MallocChecker::FreeMemAux(
-    CheckerContext &C, const Expr *ArgExpr, const Expr *ParentExpr,
     ProgramStateRef State, bool Hold, bool &IsKnownToBeAllocated,
----------------
In functions where we only use `ArgExpr` to retrieve the corresponding `SVal` we could pass the `SVal` in the first place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75432