[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG06d100a69a08: [Analyzer] Support note tags for smart ptr checker (authored by vrnithinkumar). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

NoQ accepted this revision. NoQ added a comment. Let's land this! Damn, we've learned //a lot// from this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84600/new/ https://reviews.llvm.org/D84600

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

vrnithinkumar added inline comments. 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: > vrnithinkumar wrote: > > vrnithinkumar wrote:

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

vrnithinkumar updated this revision to Diff 284237. vrnithinkumar marked an inline comment as done. vrnithinkumar added a comment. - Updating test with tags from header file Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84600/new/

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

NoQ added inline comments. Comment at: clang/test/Analysis/smart-ptr-text-output.cpp:79 + +// TODO: Enabale this test when "std::swap" is modeled seperately. +void derefOnStdSwappedNullPtr() { vrnithinkumar wrote: > vrnithinkumar wrote: > > NoQ wrote: > > >

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

vrnithinkumar added inline comments. Comment at: clang/test/Analysis/smart-ptr-text-output.cpp:79 + +// TODO: Enabale this test when "std::swap" is modeled seperately. +void derefOnStdSwappedNullPtr() { vrnithinkumar wrote: > NoQ wrote: > > Instead of commenting

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

vrnithinkumar added a comment. In D84600#2203604 , @Szelethus wrote: > Layering violations are a running theme in the analyzer -- CheckerRegistry > and the entire MallocChecker fiasco > are two glaring

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

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

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

vrnithinkumar updated this revision to Diff 284137. vrnithinkumar marked 4 inline comments as done. vrnithinkumar added a comment. - Review comment changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84600/new/ https://reviews.llvm.org/D84600

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

Szelethus added a comment. Layering violations are a running theme in the analyzer -- CheckerRegistry and the entire MallocChecker fiasco are two glaring examples. Luckily, this isn't a severe case so I wouldn't worry about it much. I've been

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

NoQ added inline comments. Comment at: clang/test/Analysis/smart-ptr-text-output.cpp:63 +void derefOnReleasedNullRawPtr() { + std::unique_ptr P; + A *AP = P.release(); // expected-note {{'AP' initialized to a null pointer value}} Unlike the next line, this

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

NoQ added inline comments. 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

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM, thanks! I think all the problems that left should be solved in a separate patch or even out of the scope for the GSoC. Comment at:

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

vrnithinkumar added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:312 + const NoteTag *getNoteTag( + std::function Cb, + bool IsPrunable = false) { xazax.hun wrote: > The callback is taken is an

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

vrnithinkumar updated this revision to Diff 283602. vrnithinkumar marked 13 inline comments as done. vrnithinkumar added a comment. - Changes from review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84600/new/

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp:110 + SmartPtrChecker *Checker = Mgr.registerChecker(); + NullDereferenceBugTypePtr = >NullDereferenceBugType; } vrnithinkumar wrote: > xazax.hun wrote: > > NoQ

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

vrnithinkumar added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp:110 + SmartPtrChecker *Checker = Mgr.registerChecker(); + NullDereferenceBugTypePtr = >NullDereferenceBugType; } xazax.hun wrote: > NoQ wrote: > >

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:199 +} else { + const auto *TrackingExpr = getFirstArgExpr(Call); + C.addTransition(State, C.getNoteTag([ThisRegion, TrackingExpr]( xazax.hun wrote: > I

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

xazax.hun added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:312 + const NoteTag *getNoteTag( + std::function Cb, + bool IsPrunable = false) { The callback is taken is an rvalue reference in

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

NoQ added a comment. Generally i think this is shaping up nicely, with the couple of minor nits addressed i'm happy to land this. Like, there are a few cornercases that we could address but we could do that in follow-up patches. Folks, do you see anything else here? Comment

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp:110 + SmartPtrChecker *Checker = Mgr.registerChecker(); + NullDereferenceBugTypePtr = >NullDereferenceBugType; } vrnithinkumar wrote: > NoQ wrote: > > Wait, i don't

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

vrnithinkumar added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp:110 + SmartPtrChecker *Checker = Mgr.registerChecker(); + NullDereferenceBugTypePtr = >NullDereferenceBugType; } NoQ wrote: > Wait, i don't understand

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/NullDereference.h:21 +namespace ento { +namespace nullDereference { + Namespaces are traditionally snake_case rather than camelCase. Comment at:

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

vrnithinkumar added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp:86 + if (const auto *DR = dyn_cast(DerefRegion)) { +auto SmartPtrName = DR->getDecl()->getName(); +OS << " '" << SmartPtrName << "'"; NoQ wrote: >

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

vrnithinkumar updated this revision to Diff 282771. vrnithinkumar marked 10 inline comments as done. vrnithinkumar edited the summary of this revision. vrnithinkumar added a comment. - Addressing review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:408-412 +SmallString<128> Msg; +llvm::raw_svector_ostream Out(Msg); +TagDetails.trackValidExpr(BR); +TagDetails.explainSmartPtrAction(Out); +

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:408-412 +SmallString<128> Msg; +llvm::raw_svector_ostream Out(Msg); +TagDetails.trackValidExpr(BR); +TagDetails.explainSmartPtrAction(Out); +

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

vrnithinkumar added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:408-412 +SmallString<128> Msg; +llvm::raw_svector_ostream Out(Msg); +TagDetails.trackValidExpr(BR); +TagDetails.explainSmartPtrAction(Out); +

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:408-412 +SmallString<128> Msg; +llvm::raw_svector_ostream Out(Msg); +TagDetails.trackValidExpr(BR); +TagDetails.explainSmartPtrAction(Out); +

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:408-412 +SmallString<128> Msg; +llvm::raw_svector_ostream Out(Msg); +TagDetails.trackValidExpr(BR); +TagDetails.explainSmartPtrAction(Out); +

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

vrnithinkumar added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:408-412 +SmallString<128> Msg; +llvm::raw_svector_ostream Out(Msg); +TagDetails.trackValidExpr(BR); +TagDetails.explainSmartPtrAction(Out); +

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:179 + + switch (SmartPtrMethod) { + case Constructor: { NoQ wrote: > I feel this is a bit over-engineered. There's no need to create an enum and > later convert it

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp:86 + if (const auto *DR = dyn_cast(DerefRegion)) { +auto SmartPtrName = DR->getDecl()->getName(); +OS << " '" << SmartPtrName << "'"; Please `getNameAsString()`

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

vrnithinkumar created this revision. Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: clang. Repository: rG LLVM Github Monorepo