vrnithinkumar marked an inline comment as done.
vrnithinkumar added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:202-219
+ProgramStateRef
+SmartPtrModeling::updateTrackedRegion(const CallEvent &Call, CheckerContext &C,
+ const MemRegion *ThisValRegion) const {
+ ProgramStateRef State = C.getState();
+ auto NumArgs = Call.getNumArgs();
+
+ if (NumArgs == 0) {
----------------
NoQ wrote:
> Szelethus wrote:
> > Hmm, this function feels clunky. So, if the call has no arguments, we set
> > the smart pointer to null, otherwise if its a single-argument then we set
> > it to whatever the argument is?
> >
> > How about `operator[]`, that also takes a single argument, but isn't a
> > memory region? `get`, `get_deleter` don't take any arguments, but they
> > don't set the internal pointee to null either. The name
> > `updateTrackedRegion` however suggests that whatever operation was done,
> > this is the one-tool-to-solve-it-all function to take care of it.
> >
> > I think this function handles too many things as once, and the name and
> > lack of documentation obfuscates its purpose. How about we put the relevant
> > code to `handleRelease`, and repurpose the rest of the function like this:
> >
> > `updateOwnedRegion(CallEvent, CheckerContext, MemRegion of the smart
> > pointer, MemRegion to take ownership of)`
> >
> > What do you think?
> Yup, I completely agree. I think this structure will naturally evolve into
> something cleaner once more modeling gets added.
Thanks for the detailed comment.
I totally agree this method is handling many things.
For methods like `get`, `get_deleter` this makes no sense at all.
I will add TODO there, will address this in a later patch once we get a clear
picture after more modeling added.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81315/new/
https://reviews.llvm.org/D81315
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
