[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

martong added a comment. Herald added a subscriber: DenisDvlp. In D74973#1926694 , @Szelethus wrote: > Please have my post-commit approval :^) Nice work! Thanks! :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

Szelethus added a comment. Please have my post-commit approval :^) Nice work! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74973/new/ https://reviews.llvm.org/D74973 ___ cfe-commits mailing list cfe-c

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

This revision was automatically updated to reflect the committed changes. Closed by commit rGc6b8484e855b: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74973/new/

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

martong marked 12 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:88 typedef uint32_t ArgNo; - static const ArgNo Ret = std::numeric_limits::max(); + static const ArgNo Ret; + --

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

martong updated this revision to Diff 250732. martong marked 3 inline comments as done. martong added a comment. - Rebase to master - Add comments to ValueConstraint - Add virtual dtor - Add comments to ValueConstraintPtr Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. I like this! Please address the remaining nits^^ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:102 + /// Given a range, should the argument stay ins

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

baloghadamsoftware added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:102 + /// Given a range, should the argument stay inside or outside this range? + enum RangeKind { OutOfRange, WithinRange }; I would move t

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:151 + + using ValueConstraintPtr = std::shared_ptr; + /// The complete list of constraints that defines a single branch. martong wrote: > Szelethus wro

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

martong marked 2 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:151 + + using ValueConstraintPtr = std::shared_ptr; + /// The complete list of constraints that defines a single branch. ---

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

Szelethus added a comment. Ah, okay I think I got why you chose this direction. Summaries are little more then a collection of constraints, and at select points in the execution we check and apply them one-by-one. If we want to preserve this architecture (and it seems great, why not?), inherita

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

martong marked 4 inline comments as done. martong added a comment. In D74973#1900852 , @Szelethus wrote: > I have some high level questions, you have spent far more time with this code > and I'm happy to be proven wrong! :) > > In D74973#1889188

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

Szelethus added a comment. I have some high level questions, you have spent far more time with this code and I'm happy to be proven wrong! :) In D74973#1889188 , @martong wrote: > > Is really more kind of constraint needed than range constraint? > > Yes,

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

gamesh411 added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:151 + + using ValueConstraintPtr = std::shared_ptr; + /// The complete list of constraints that defines a single branch. martong wrote: > Note here, we

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

martong added a comment. @balazske See https://reviews.llvm.org/D75063 about the simple independent implementation of the NotNull constraint. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74973/new/ https://reviews.llvm.org/D74973 _

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

martong marked an inline comment as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:93 +ValueConstraint(ArgNo ArgN) : ArgN(ArgN) {} +virtual ProgramStateRef apply(ProgramStateRef State, const CallEvent &Cal

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

martong added a comment. In D74973#1889273 , @Szelethus wrote: > FYI I've been seeing your patches to this checker and I will respond to them, > but I need to do some learning on my own before having the confidence to > accept or request changes. Working

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

Szelethus added a comment. FYI I've been seeing your patches to this checker and I will respond to them, but I need to do some learning on my own before having the confidence to accept or request changes. Working on it! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

martong added a comment. > Is really more kind of constraint needed than range constraint? Yes, there are other constraints I am planning to implement: - Size requirements E.g.: asctime_s(char *buf, rsize_t bufsz, const struct tm *time_ptr); `buf` size must be at least `bufsz`. - Not-null - Not

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

balazske added a comment. Is really more kind of constraint needed than range constraint? A non-null can be represented as range constraint too. The compare constraint is used only for the return value for which a special `ReturnConstraint` can be used to handle the return value not like a norm

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

martong marked an inline comment as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:151 + + using ValueConstraintPtr = std::shared_ptr; + /// The complete list of constraints that defines a single branch. ---

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

martong created this revision. martong added reviewers: NoQ, Szelethus, balazske, gamesh411, baloghadamsoftware, steakhal. Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun, whisperity. Herald added a project: clang. marto