[PATCH] D68163: [analyzer][MallocChecker][NFC] Change the use of IdentifierInfo* to CallDescription

Sat Sep 28 02:21:20 PDT 2019

Szelethus marked 2 inline comments as done.
Szelethus added inline comments.

================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:259
   /// calls.
   bool isCalled(const CallDescription &CD) const;

----------------
NoQ wrote:
> I don't fully understand how does overload resolution work in this case, maybe rename the original function?
Well, `isCalled` may now have an arbitrary number of arguments. The single argument version is this function, and the //n//-argument ones call this //n// times: `isCalled(A, B, C)` -> `isCalled(A) || isCalled(B) || isCalled(B)`. I guess I could rename the other one to `isAnyCalled`, but I think its fine.

================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1006
   const FunctionDecl *FD = C.getCalleeDecl(CE);
-  if (!FD)
+  if (!FD || FD->getKind() != Decl::Function)
     return;
----------------
kimgr wrote:
> NoQ wrote:
> > The `FD->getKind() != Decl::Function` part is super mega redundant here.
> Sorry for jumping in from nowhere. AFAIK, this is the only way to detect free vs member functions. It looks like this wants to discard member functions. Are you sure it's redundant? 
Please do! Though I have a suspicion that if this isn't redundant, such a check should be done in `CallDescription`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68163