[PATCH] D124845: StaticAnalyzer should inline overridden delete operator the same way as overridden new operator

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Looks promising, I am close to accept this, thanks!



================
Comment at: clang/include/clang/Analysis/ConstructionContext.h:122-124
     assert(isa<CallExpr>(E) || isa<CXXConstructExpr>(E) ||
-           isa<CXXInheritedCtorInitExpr>(E) || isa<ObjCMessageExpr>(E));
+           isa<CXXDeleteExpr>(E) || isa<CXXInheritedCtorInitExpr>(E) ||
+           isa<ObjCMessageExpr>(E));
----------------
I think we could use the variadic `isa` template .


================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:950
 
-  getCheckerManager().runCheckersForPostCall(Dst, DstPreCall, *Call, *this);
+  if (AMgr.getAnalyzerOptions().MayInlineCXXAllocator) {
+    StmtNodeBuilder Bldr(DstPreCall, DstPostCall, *currBldrCtx);
----------------
Could you please also update the config option's documentation at `AnalyzerOptions.def`?


================
Comment at: clang/test/Analysis/cxxnewexpr-callback-inline.cpp:1
 // RUN: %clang_analyze_cc1 -analyzer-checker=debug.AnalysisOrder -analyzer-config c++-allocator-inlining=true,debug.AnalysisOrder:PreStmtCXXNewExpr=true,debug.AnalysisOrder:PostStmtCXXNewExpr=true,debug.AnalysisOrder:PreCall=true,debug.AnalysisOrder:PostCall=true,debug.AnalysisOrder:NewAllocator=true %s 2>&1 | FileCheck %s
 
----------------
It is hard to see how the two test files are different.
Ideally, the two test files `cxxnewexpr-callback-inline.cpp` and `cxxnewexpr-callback-noinline.cpp` could be merged into the very same test file with two RUN lines and with different `--check-prefix`. 

However, I see that this is kind of unrelated to this particular change, so, that could be done in a separate, follow-up patch, if you have the time and mood for that.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124845