[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

This revision was automatically updated to reflect the committed changes. Closed by commit rGe935a540ea29: [Analyzer][StreamChecker] Add note tags for file opening. (authored by balazske). Changed prior to commit: https://reviews.llvm.org/D81407?vs=271291&id=272355#toc Repository: rG LLVM Gi

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

NoQ accepted this revision. NoQ added a comment. Yup, i think so! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81407/new/ https://reviews.llvm.org/D81407 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

balazske added a comment. So for this patch it would be OK to have the uniqueing location as it is now. A next large change can be to add the global resource leak report uniqueing feature, this changes anyway more existing checkers (including this one). (Still I want to finish other improvement

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

NoQ added a comment. In D81407#2102951 , @Szelethus wrote: > That could be helped additionally by creating a distinct `LeakBugReport`, > derived from `PathSensitiveBugReport`, that would take non-optional uniqueing > lambda to find the `ExplodedNode` res

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

Szelethus added a comment. I see where you're coming from @NoQ. What do you think, @balazske? I think there is is still value in this implementation as a //debug// option to gather data, so that we don't invest a lot of time creating a robust infrastructure for an idea that might not work out.

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

balazske added a comment. I do not understand fully this "globally". A new option should be added that affects all checkers that detect some kind of resource leak? And then implement that kind of report uniqueness in all checkers that detect resource leak. Other possible solution: Leave the cur

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

NoQ added a comment. > Balázs, could you please add the checker option within this patch? I'd rather have this decision made globally. Like, for all //leaks//, or something like that. Our behavior should be consistent. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

Szelethus added a comment. Balázs, could you please add the checker option within this patch? If we find that the option works well (removes a lot of useless reports) I'd be happy to help implement that uniqueing pass. CmdLineOption, Comment at: clang/lib/StaticAnalyzer/Ch

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:406 +const ExplodedNode *StreamChecker::getAcquisitionSite(const ExplodedNode *N, + SymbolRef StreamSym, balazske wro

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

balazske marked 3 inline comments as done. balazske added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:406 +const ExplodedNode *StreamChecker::getAcquisitionSite(const ExplodedNode *N, + Sy

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:406 +const ExplodedNode *StreamChecker::getAcquisitionSite(const ExplodedNode *N, + SymbolRef StreamSym, NoQ wrote: >

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:406 +const ExplodedNode *StreamChecker::getAcquisitionSite(const ExplodedNode *N, + SymbolRef StreamSym, Ok, so this

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

Szelethus accepted this revision. Szelethus marked 2 inline comments as done. Szelethus added a comment. This revision is now accepted and ready to land. Yay! Getting so close to enabling this by default. I'm a big fan of your work on this checker. Comment at: clang/lib/Static

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

balazske updated this revision to Diff 271291. balazske marked 2 inline comments as done. balazske added a comment. Corrected command line arguments in tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81407/new/ https://reviews.llvm.org/D81407

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

balazske updated this revision to Diff 271007. balazske marked an inline comment as done. balazske added a comment. - Re-added the location uniqueing feature. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81407/new/ https://reviews.llvm.org/D81407

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

balazske updated this revision to Diff 270974. balazske added a comment. - Rebase - Added check for checker in NoteTag function. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81407/new/ https://reviews.llvm.org/D81407 Files: clang/lib/StaticAnal

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

Szelethus added a comment. I'd still like to see more `NoteTag`s such as "File read failed, end-of-file indicator set on 'F'", and a final evaluation would be nice, but otherwise this checker looks amazing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

NoQ added a reviewer: NoQ. NoQ added a comment. Do i understand correctly that the checker is no longer "missing limbs" and we should consider turning it on by default? If so, @balazske could you prioritize hunting down the remaining false positives above adding new checks / hunting down false

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:376-377 +std::string operator()(PathSensitiveBugReport &BR) const { + if (BR.isInteresting(StreamSym)) +return Message; + Another thing you might want to c

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

Szelethus added a comment. In D81407#2087624 , @balazske wrote: > - Report every path of resource leak. I thought we agreed on the uniqueing being great? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81407

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

balazske updated this revision to Diff 270161. balazske added a comment. - Added tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81407/new/ https://reviews.llvm.org/D81407 Files: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp clang/t

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

balazske updated this revision to Diff 270140. balazske added a comment. - Report every path of resource leak. - Do not report if non-returning function was encountered. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81407/new/ https://reviews.llvm.

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

Szelethus added a reviewer: xazax.hun. Szelethus added a comment. Herald added a subscriber: rnkovacs. Alright, I'm sold. How about we add a checker option for it? I don't actually insist, just an idea. @xazax.hun, how has this feature played out? Comment at: clang/lib/StaticA

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

balazske added a comment. In D81407#2080273 , @Szelethus wrote: > > If there are multiple resource leak paths for the same stream, only one wil > > be reported. > > What is the rationale behind this? Do all leaks from the same opening > describe the same

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

Szelethus added a comment. > If there are multiple resource leak paths for the same stream, only one wil > be reported. What is the rationale behind this? Do all leaks from the same opening describe the same kind of error? Is this based on observations on a codebase? I'm not against it -- but

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

balazske created this revision. Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: Szelethus. Herald added a project: clang. Bug reports of