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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
