NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Sorry i'm not very responsive these days >.<
================
Comment at:
clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp:95
ExplodedNode *N = C.generateNonFatalErrorNode();
+ if (!N)
+ return;
----------------
xazax.hun wrote:
> 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.
I'm also curious why did this suddenly become necessary. This is obviously the
right thing to do but why the change? It might indicate that we accidentally
started incorrectly agglutinating nodes that were previously considered
different. Like, we might have messed up our program points somewhere in this
patch.
================
Comment at: clang/test/Analysis/cxx-dynamic-memory-analysis-order.cpp:31-32
- // FIXME: All calls to operator new should be CXXAllocatorCall.
- // FIXME: PostStmt<CXXDeleteExpr> should be present.
+ // FIXME: All calls to operator new should be CXXAllocatorCall, and calls to
+ // operator delete should be CXXDeallocatorCall.
{
----------------
That's fairly unobvious to me. Isn't the whole point of
`CXXAllocatorCall`/`CXXDeallocatorCall` to give access to specific aspects of
new/delete-expressions that aren't available in manual invocations with
function syntax? On the other hand, i agree that it's nice to be able to cast
the call event to `CXXAllocatorCall`/`CXXDeallocatorCall` and be done with
allocators/deallocators.
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
