[PATCH] D108753: [analyzer] MallocChecker: Add notes from NoOwnershipChangeVisitor only when a function "intents", but doesn't change ownership, enable by default

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:802
+    // TODO: Operator delete is hardly the only deallocator -- Can we reuse
+    // isFreeingCall() or something thats already here?
+    auto Deallocations = match(
----------------
Sounds like a good idea to reuse `isFreeingCall`. 

But interestingly `delete` is not listed there! Nor `new` in `AllocatingMemFnMap`. So, perhaps we need yet another abstraction. Maybe a function that iterates over `FreeingMemFnMap` collects the functions and then adds `delete` as well?


================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:806
+             ), *FD->getBody(), ACtx);
+    // TODO: Ownership my change with an attempt to store the allocated memory.
+    return !Deallocations.empty();
----------------
Good point!


================
Comment at: clang/test/Analysis/NewDeleteLeaks.cpp:23
 
+// TODO: AST analysis of sink would reveal that it doesn't indent to free the
+// allocated memory, but in this instance, its also the only function with
----------------
intent

BTW, why is this TODO here? It is obvious that `sink` does not have a `delete`, so we don't expect any warning. Or am I missing something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108753