[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

balazske abandoned this revision. balazske added a comment. The patch was split into D75612 and D75614 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75163/new/ https://reviews.ll

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. Okay, very well, then lets abandon this patch. Please keep in mind that phabricator can mark whether a revision was mentioned somewhere else if you only copy the revision ID, n

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

balazske added a comment. To avoid problems I created a new version of this diff too that follows after the other new ones: https://reviews.llvm.org/D75682 (Adding a new diff can make inline comment positions even more inexact specially if the diff has many differences from an older one?) Repo

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:64-70 +/// Get the value of the stream argument out of the passed call event. +/// The call should contain a function that is described by Desc. +SVal getStreamArg(const FnDescription

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

balazske added a comment. And the second part: https://reviews.llvm.org/D75614 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75163/new/ https://reviews.llvm.org/D75163 ___ cfe-commits mailing list cfe-

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

balazske added a comment. I had no other work to do so here is the change for the non-functional part of this: https://reviews.llvm.org/D75612 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75163/new/ https://reviews.llvm.org/D75163

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

Szelethus added a comment. In D75163#1902816 , @xazax.hun wrote: > The name of the patch implies refactoring but some functional changes were > also done. Is it possible to separate the functional changes into a separate > patch? If it is not a big effor

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

xazax.hun added a comment. In D75163#1902921 , @balazske wrote: > In D75163#1902816 , @xazax.hun wrote: > > > If we were to refactor this check I wonder if it would make sense to port > > `evalCall` to `postCall`,

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

balazske added a comment. Split the patch into two is really start it from "scratch" again, copy the code, decide what to go into the first or second part (and what order) (really what?), make some mess with git commands and branches and rebase, probably rebuild whole llvm, if review changes ar

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

balazske added a comment. In D75163#1902816 , @xazax.hun wrote: > If we were to refactor this check I wonder if it would make sense to port > `evalCall` to `postCall`, so the analyzer engine will conjure the symbol for > us. > I wonder what @NoQ thinks

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

xazax.hun added a comment. If we were to refactor this check I wonder if it would make sense to port `evalCall` to `postCall`, so the analyzer engine will conjure the symbol for us. I wonder what @NoQ thinks about the pros and cons of the approaches. As far as I understand the con of evalCall is

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

balazske updated this revision to Diff 247847. balazske marked an inline comment as done. balazske added a comment. Comment changes, updated `lookupFn`, other small change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75163/new/ https://reviews.ll

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

balazske marked 6 inline comments as done. balazske added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:64-70 +/// Get the value of the stream argument out of the passed call event. +/// The call should contain a function that is described by De

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. LGTM! It makes much more sense to do the checking in `preCall` and the modeling in `evalCall`, the comments in the patch are precise and helpful, the tests cover everything. While I feel

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

balazske updated this revision to Diff 247192. balazske marked 2 inline comments as done. balazske added a comment. Added a missing argument, lint warning reformat. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75163/new/ https://reviews.llvm.org/D

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

balazske marked 10 inline comments as done. balazske added a comment. Error messages are now in similar form as the null stream pointer case. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:76-92 + {{"fopen"}, {{}, &StreamChecker::evalFopen, ArgNone}}, +

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

balazske updated this revision to Diff 246955. balazske added a comment. - Improved comments in the code. - Added function to get stream arg. - Changed error messages. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75163/new/ https://reviews.llvm.or

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

Szelethus added a comment. To summarize, we're moving every stream check (illegal call to a stream management function with a null stream, or a closed stream, etc) to `preCall`, and leave only the modeling portions in `evalCall`. Makes sense! You did an amazing job here! I like the new infrastr

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

balazske added a comment. Plan: - Add an error state to `StreamState`. - Model every stream function that can fail. Split the state into failed and non-failed and set the return value accordingly (as done at //fopen// now, but not only for stream return values). This is needed here to have a co

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

balazske updated this revision to Diff 246711. balazske added a comment. Really reformat, bugfixes, and updated tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75163/new/ https://reviews.llvm.org/D75163 Files: clang/lib/StaticAnalyzer/Check

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

balazske updated this revision to Diff 246687. balazske added a comment. Reformat code and fixed errors. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75163/new/ https://reviews.llvm.org/D75163 Files: clang/lib/StaticAnalyzer/Checkers/StreamChec

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

Szelethus added a comment. Aha, so we're moving every check as to whether the stream is closed to `preCall`? Makes sense, if not much else changed. Could you please run `clang-format-diff.py` after adding the tests? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

balazske added a comment. Code is not reformatted and tests are to be added. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75163/new/ https://reviews.llvm.org/D75163 ___ cfe-commits mailing list cfe-co

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

balazske created this revision. Herald added subscribers: cfe-commits, 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. The validity checks are perfor