vrnithinkumar added inline comments.

================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp:33
 
+// Static global pointer to NullDereferenceBugType.
+static const BugType *NullDereferenceBugTypePtr;
----------------
xazax.hun wrote:
> I find this comment redundant as well. It just repeats what is already 
> evident from the code.
removed


================
Comment at: clang/test/Analysis/smart-ptr-text-output.cpp:63
+void derefOnReleasedNullRawPtr() {
+  std::unique_ptr<A> P;
+  A *AP = P.release(); // expected-note {{'AP' initialized to a null pointer 
value}}
----------------
NoQ wrote:
> Unlike the next line, this line deserves a note that `P` holds a null value.
Added a FIXEM to add note "Default constructed smart pointer 'P' is null"


================
Comment at: clang/test/Analysis/smart-ptr-text-output.cpp:65
+  A *AP = P.release(); // expected-note {{'AP' initialized to a null pointer 
value}}
+  //TODO add note "Smart pointer 'P' is released and set to null"
+  AP->foo(); // expected-warning {{Called C++ object pointer is null 
[core.CallAndMessage]}}
----------------
NoQ wrote:
> Such note is unnecessary. We don't care what happens to `P` after it's 
> released; we only care about its old value.
Removed this.


================
Comment at: clang/test/Analysis/smart-ptr-text-output.cpp:79
+
+// TODO: Enabale this test when "std::swap" is modeled seperately.
+void derefOnStdSwappedNullPtr() {
----------------
NoQ wrote:
> Instead of commenting out tests, i recommend testing the incorrect behavior 
> (with a FIXME comment telling us why it's incorrect). This way we'll be 
> notified when the test is fixed, accidentally or intentionally, and also 
> generally that's more testing for everybody.
I have commented out this since right now `std::swap` is using 
`unique_ptr.swap`.
So note tag for std::swap is added in header file  
`system-header-simulator-cxx.h`
eg:
`system-header-simulator-cxx.h Line 978: Swapped null smart pointer 'PNull' 
with smart pointer 'P'`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84600

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

Reply via email to