Szelethus requested review of this revision.
Szelethus added a comment.
Since this change had a few blocking issues, I'm placing it back to review for
greater visibility.
================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:67
CE_CXXAllocator,
+ CE_CXXDeallocator,
CE_BEG_FUNCTION_CALLS = CE_Function,
----------------
NoQ wrote:
> After this you probably received compiler warnings saying "case isn't covered
> in switch". You'll need to clean them up.
>
> Another thing to do would be to update `CallEventManager`'s `getCall()` and
> `getCaller()` methods that'd allow the users to construct the new `CallEvent`
> easily without thinking about what specific kind of call event it is.
No, I did not, infuriatingly. I did however get //errors// after trying to make
a `toString` function for `CallEventKind`, apparently both `CE_CXXDeallocator`
and `CE_Block` has the value of 7. When I moved the enum, everything was fine,
and I did get the warnings you mentioned.
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:958
+
+ unsigned getNumArgs() const override { return getDecl()->getNumParams(); }
+
----------------
steakhal wrote:
> NoQ wrote:
> > Charusso wrote:
> > > `return getOriginExpr()->getNumArgs()`
> > This wouldn't compile. `CXXDeleteExpr` is not a `CallExpr`.
> >
> > It sounds like there are currently [[
> > http://www.cplusplus.com/reference/new/operator%20delete/ | five different
> > `operator delete`s ]]:
> > {F11474783}
> >
> > And, unlike `operator new`, there's no option to provide custom "placement"
> > arguments.
> >
> > So i think the logic in this patch is correct but we should do some custom
> > logic for all 5 cases in the `getArgExpr` method, where argument
> > expressions for the extra arguments will have to be conjured out of thin
> > air (or we might as well return null).
> > It sounds like there are currently five different `operator delete`s:
> I think it is even worse since C++17 and C++20 introduced a couple of others
> like:
> - overloads with `std::align_val_t` parameter (C++17)
> - overloads with `std::destroying_delete_t` parameter (C++20) which I
> haven't heard of yet :D
>
> You can check it in the draft: http://eel.is/c++draft/new.delete
> And of course at cppreference as well:
> https://en.cppreference.com/w/cpp/memory/new/operator_delete
Okay so here is the biggest issue: non-simple `delete`s don't appear in the AST
as `CXXDeleteExpr`, but rather as a `CXXOperatorCall`. This is a big problem,
what could be the reason for it?
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
