[PATCH] D75430: [analyzer][NFC] Introduce CXXDeallocatorCall, deploy it in MallocChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Sorry i'm not very responsive these days >.<



================
Comment at: clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp:95
   ExplodedNode *N = C.generateNonFatalErrorNode();
+  if (!N)
+    return;
----------------
xazax.hun wrote:
> I think this change might be a better fit for a separate commit. I think you don't even need to have such a small change reviewed. You could just commit it as is. Just do not forget to describe that these cases are really hard to have a test for but this is the idiomatic way of doing this in checkers.
I'm also curious why did this suddenly become necessary. This is obviously the right thing to do but why the change? It might indicate that we accidentally started incorrectly agglutinating nodes that were previously considered different. Like, we might have messed up our program points somewhere in this patch.


================
Comment at: clang/test/Analysis/cxx-dynamic-memory-analysis-order.cpp:31-32
 
-  // FIXME: All calls to operator new should be CXXAllocatorCall.
-  // FIXME: PostStmt<CXXDeleteExpr> should be present.
+  // FIXME: All calls to operator new should be CXXAllocatorCall, and calls to
+  // operator delete should be CXXDeallocatorCall.
   {
----------------
That's fairly unobvious to me. Isn't the whole point of `CXXAllocatorCall`/`CXXDeallocatorCall` to give access to specific aspects of new/delete-expressions that aren't available in manual invocations with function syntax? On the other hand, i agree that it's nice to be able to cast the call event to `CXXAllocatorCall`/`CXXDeallocatorCall` and be done with allocators/deallocators.


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

https://reviews.llvm.org/D75430