vrnithinkumar added inline comments.

================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:619
       getCheckerManager().runCheckersForEvalCall(DstEvaluated, *I, *Call, 
*this,
-                                                 CallOpts);
+                                                 CallOpts, Bldr);
   }
----------------
NoQ wrote:
> We should probably delete the copy-constructor for node builders. I've no 
> idea what it's supposed to do anyway and the whole problem that we're having 
> here is due to there being //too many// of them already.
So we should disable the copying of `NodeBuilder` and create a heap allocated 
`NodeBuilder` and use pointer to pass around functions?


================
Comment at: clang/test/Analysis/smart-ptr-text-output.cpp:39
 void derefAfterRelease() {
-  std::unique_ptr<A> P(new A());
+  std::unique_ptr<A> P(new A()); // expected-note {{Smart pointer 'P' is 
constructed}}
   P.release(); // expected-note {{Smart pointer 'P' is released and set to 
null}}
----------------
NoQ wrote:
> Ok, these notes shouldn't be there; a note on `.release()` is sufficient to 
> understand the warning and it looks like that's one more place where we 
> should mark the region as uninteresting.
> 
> Can you try to debug why did they suddenly show up?
I checked the exploded graph for this test case. 
Before the bug fix, there exists a path where the no Note Tag is added to the 
corresponding `CXXConstructExpr`. After the fix removed this branching theres 
always a Note Tag on Ctr. {F12591752}

Since the note on .release() is sufficient to understand the warning and I 
agree we should mark this region as uninteresting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85796

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

Reply via email to