xazax.hun added a comment. Overall looks good to me some questions and nits inline.
================ Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:59 enum CallEventKind { + CE_CXXDeallocator, CE_Function, ---------------- Szelethus wrote: > I need to move this below `CE_Function`, as its not in the range of > `CE_BEG_FUNCTION_CALLS` and `CE_END_FUNCTION_CALLS`. This is marked as done. If the conclusion is that you do not need to move it I think it would be worth to add a comment why :) ================ Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:1052 +/// This is a call to "operator delete". +// FIXME: CXXDeleteExpr isn't present for custom delete operators, or even for +// some those that are in the standard library, like the no-throw or align_val ---------------- How important and hard it is to fix this? If you did not find the reason why those CXXDeleteExprs are missing you can do two things: 1. Look at how such AST is consumed by CodeGen 2. Ask around on the mailing list with a minimal example. ================ Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:1083 + + const Expr *getArgExpr(unsigned Index = 0) const override { + return getOriginExpr()->getArgument(); ---------------- Is the index unused on purpose? If yes, wouldn't this trigger a warning on build bots? Adding a maybe unused annotation and/or a comment would be welcome. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp:95 ExplodedNode *N = C.generateNonFatalErrorNode(); + if (!N) + return; ---------------- 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. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75430/new/ https://reviews.llvm.org/D75430 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits