NoQ added a comment. Yup, this makes sense now! I'll do some nit-picking for a little longer.
================ Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:839 + // as a 'StmtPoint' so we have to bypass it. + const bool IsBypass = isa<CXXNewExpr>(S); + ---------------- For now this hack is clearly only for CXXNewExpr. And it happens because our AST is weird and there's no separate sub-expression dedicated entirely to invoking the allocator. Therefore let's give this variable a better name, maybe `BypassCXXNewExprEvaluation` or something like that. ================ Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:856-860 + if (Optional<CallExitBegin> CEB = Node->getLocationAs<CallExitBegin>()) + CurrentSFC = CEB->getStackFrame(); + + if (Optional<CallEnter> CE = Node->getLocationAs<CallEnter>()) + CurrentSFC = CE->getStackFrame(); ---------------- Branches here are unnecessary because there's no performance win here. Let's just ```lang=c++ CurrentSFC = Node->getStackFrame(); ``` ================ Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:862-866 + // If that is satisfied we found our statement. + if (!IsBypass) + if (Optional<StmtPoint> SP = Node->getLocationAs<StmtPoint>()) + if (SP->getStmt() == S && SP->getStackFrame() == CurrentSFC) + break; ---------------- This code would currently bypass the call site for the conservatively evaluated allocator. It doesn't seem to be immediately important but let's add a FIXME. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62926/new/ https://reviews.llvm.org/D62926 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
