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