Szelethus added a comment.

In D105553#2867881 <https://reviews.llvm.org/D105553#2867881>, @NoQ wrote:

> In the mailing list I seem to have made a mistake about how this works: we 
> don't explicitly scan the AST for potential statements that could cause a 
> state change (eg., assignment operators in case of `NoStore`) but we only 
> check if the region was explicitly made accessible. This makes things a lot 
> easier (we don't have to build an auxiliary AST-based analysis) and I'm not 
> sure if this other heuristic that I thought we have would actually make 
> things significantly better. I guess it makes sense to keep in mind that we 
> might want to ultimately plug it in but I don't immediately see what'd stop 
> us.

For `NoStoreFuncVisitor`, this seems to be fine I suppose, though I'm not sure 
how things pan out for memory leaks. As you put it,

> We probably shouldn't emit a note every time the function simply accepts the 
> value of interest by pointer, because this doesn't necessarily prove the 
> intent to delete the pointer;

it might be rather difficult to tell this intent. For now, I'm leaning on the 
side of slightly too much than too little, but I need real world data on my 
hand to have a good feel for it.



================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:399
+/// to get the correct parameter name.
+static ArrayRef<ParmVarDecl *> getCallParameters(const CallEvent &Call) {
+  // Use runtime definition, if available.
----------------
NoQ wrote:
> While we're at it, can you take a look if the recently introduced 
> `CallEvent::parameters()` is good enough for this?
I suppose yeah, at least the lit tests all passed, though its worth mentioning 
that this function explicitly calls `CallEvent::parameters()`...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105553/new/

https://reviews.llvm.org/D105553

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to