[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

2021-05-27 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtr.h:37 + +class GetNoteVisitor : public BugReporterVisitor { +private: vsavchenko wrote: > RedDocMD wrote: > > vsavchenko wrote: > > > Sorry for picking on it again, but is it

[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

2021-05-27 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtr.h:36 + +class FindWhereConstrained : public BugReporterVisitor { +private: RedDocMD wrote: > vsavchenko wrote: > > vsavchenko wrote: > > > nit: class name should be a noun (f

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

2021-05-28 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 348479. vsavchenko added a comment. Fix IE issue and rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92639/new/ https://reviews.llvm.org/D92639 Files: clang/include/clang/Analysis/PathDiagnostic.h

[PATCH] D92928: [analyzer] Highlight arrows for currently selected event

2021-05-28 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 348480. vsavchenko added a comment. Herald added a subscriber: manas. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92928/new/ https://reviews.llvm.org/D92928 Files: clang/lib/Rewrite/HTMLRewrite.c

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

2021-05-28 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D92639#2785057 , @ASDenysPetrov wrote: > @vsavchenko How about this? Yep, thanks for reminding! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92639/new/ https://reviews.llvm

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

2021-05-28 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D92639#2786648 , @ASDenysPetrov wrote: > OK, then. Let's land it! Can you please take a look at D92928 as well? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

2021-05-28 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko requested changes to this revision. vsavchenko added a comment. This revision now requires changes to proceed. Hey, great job! This is really something that we need, but it's implemented not really correctly. I tried to cover it in the inline comment. Comment at: cl

[PATCH] D103317: [Analyzer][engine][solver] Simplify complex constraints

2021-05-28 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. Looking great, thanks! I have a couple of notes below. Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1155-1156 -// TODO: Support SymbolCast. Support IntSymExpr when/if we actually -// start producing them. --

[PATCH] D102696: [Analyzer] Find constraints that are directly attached to a BinOp

2021-05-28 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. Oh, wow. Great plan! I'd like to participate. 😊 I have a few comments: > Alright, I see your point. I agree that solving the problem of "$a +$b +$c > constrained and then $a + $c got constrained" requires canonicalization. That was actually not an example for canonica

[PATCH] D102696: [Analyzer] Find constraints that are directly attached to a BinOp

2021-05-28 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. I just noticed that I simply forgot to click submit yesterday 😩 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102696/new/ https://reviews.llvm.org/D102696 ___ cfe-commits mail

[PATCH] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

2021-05-28 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1561 + return nullptr; + +ConstraintMap CM = getConstraintMap(State); Also I think we can introduce a simple, but efficient optimization of kicking of

[PATCH] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

2021-05-31 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. That's awesome, just a few stylistic tweaks and tests and we are to land! Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1561-1562 + +if (!Constraint.getConcreteValue()) + return State; + I think we nee

[PATCH] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

2021-05-31 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1576-1577 + // Add the newly simplified symbol to the equivalence class. + State = + Class.merge(this->getBasicVals(), F, State, +

[PATCH] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

2021-05-31 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. I had another thought, `merge` is usually called in situations when we found out that two symbols should be marked equal (and checked that it's possible to begin with), which is not true in your case. If we update my case from before, we can get: `a + b == c` and `a

[PATCH] D103434: [analyzer] Allow visitors to run callbacks on completion

2021-06-01 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:895 +using VisitorCallback = llvm::function_ref; `function_ref` is a reference, it doesn't own the function. It means that it shouldn't outlive the actual funct

[PATCH] D103440: [WIP][analyzer] Introduce range-based reasoning for addition operator

2021-06-01 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. Hey, great start! I added my comments inline and other mentors as reviewers. Comment at: clang/test/Analysis/constant-folding.c:275 + +if (a <= UINT_MAX && b <= UINT_MAX) { + clang_analyzer_eval((a + b) < 0); // expected-warning{{UNKNOWN}} -

[PATCH] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

2021-06-01 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. Awesome! I know, I said that we are ready to land, but I think I was too excited about this change. We probably should have some data on how it performs on real-life codebases. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:15

[PATCH] D103457: [analyzer] Add forwarding `addVisitor` method

2021-06-01 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko created this revision. vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus, manas, RedDocMD. Herald added subscribers: ASDenysPetrov, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. vsavchenko requested review of this revision.

[PATCH] D103440: [WIP][analyzer] Introduce range-based reasoning for addition operator

2021-06-02 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103440#2792814 , @NoQ wrote: > Our pre-commit testing shows that the new tests fail, eg.: > > https://buildkite.com/llvm-project/premerge-checks/builds/41391#b557c86c-0587-4aee-a06b-8a4de6d771c4 > > Failed Tests (1): >

[PATCH] D103317: [Analyzer][engine][solver] Simplify complex constraints

2021-06-02 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1155-1156 -// TODO: Support SymbolCast. Support IntSymExpr when/if we actually -// start producing them. ASDenysPetrov wrote: > vsavchenko wrote: > > Do

[PATCH] D103317: [Analyzer][engine][solver] Simplify complex constraints

2021-06-02 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103317#2793557 , @ASDenysPetrov wrote: > Returning to the discussion raised in D102696 > , I'd like to share my vision. > I think we can use much easier approach to use valid constraints

[PATCH] D103317: [Analyzer][engine][solver] Simplify complex constraints

2021-06-02 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103317#2793781 , @ASDenysPetrov wrote: > In D103317#2793658 , @vsavchenko > wrote: > >> But the problem it is generally not one-to-one relationship, so `x -> y1 + >> 1`, `x -> y2

[PATCH] D103317: [Analyzer][engine][solver] Simplify complex constraints

2021-06-02 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103317#2793911 , @ASDenysPetrov wrote: > In D103317#2793797 , @vsavchenko > wrote: > >> Hmm, Okay, but what about situations if you have: `a = a1 + a2` and `a = a3 >> + a4 + a5`

[PATCH] D103440: [WIP][analyzer] Introduce range-based reasoning for addition operator

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. I also would like to see tests where the ranges are not going all the way to either `INT_MIN` or `INT_MAX` (if we talk about `int`), but overflow still might happen, and cases where overflow might happen, but we still can identify the overflowing results precisely (e

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko created this revision. vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus, manas, RedDocMD. Herald added subscribers: ASDenysPetrov, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. vsavchenko requested review of this revision.

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 349511. vsavchenko added a comment. Add docstring for StoreHandler Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103605/new/ https://reviews.llvm.org/D103605 Files: clang/include/clang/StaticAnalyzer/Core

[PATCH] D103434: [analyzer] Allow visitors to run callbacks on completion

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. I was thinking a lot about this problem after our last call, and even though `StoppableVisitor` helps to solve the problem that we have, it is extremely unclear when this callback is called and should be called. I decided to restore the balance (and rid of all youngli

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 349529. vsavchenko added a comment. Tweak some parts of the interface Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103605/new/ https://reviews.llvm.org/D103605 Files: clang/include/clang/StaticAnalyzer/C

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103605#2796111 , @martong wrote: > Great initiative! You haven't mention `markInteresting` functions, but I > think it would be really important to incorporate that functionality here as > well. IMHO, `markInteresting` sh

[PATCH] D92221: Don't delete default constructor of PathDiagnosticConsumerOptions

2021-01-22 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko accepted this revision. vsavchenko added a comment. Hi @MoritzS thanks for the cleanup! Maybe while you are on it, you can set default values as pointed by @NoQ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92221/new/ https://reviews.ll

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-01-25 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D95307#2518592 , @NoQ wrote: > This patch shoots the messenger but someone still needs to conduct a proper > investigation. The assertion is losing a lot more of its bug-finding power > than necessary to uncover the current

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-01-25 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D95307#2519300 , @RedDocMD wrote: > In D95307#2519283 , @vsavchenko > wrote: > >> In D95307#2518592 , @NoQ wrote: >> >>> This patch shoots the

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-01-25 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D95307#2519309 , @RedDocMD wrote: > Funnily enough, when I run `ninja clang-check` I don't get any errors. I believe that `ninja check-clang` is the right command (clang-check is a tool) :-) Repository: rG LLVM Github M

[PATCH] D93630: [Attr] Apply GNU-style attributes to expression statements

2021-01-27 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 319516. vsavchenko added a comment. Herald added a subscriber: dexonsmith. Add extension for backward compatibility checks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93630/new/ https://reviews.llvm.org/D9

[PATCH] D95519: [clang-tidy] bugprone-assert-side-effect: Warn on NSAssert by default.

2021-01-27 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. LGTM, but it would be great if code owners take a look as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95519/new/ https://reviews.llvm.org/D95519 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

[PATCH] D93110: [analyzer] Implement fine-grained suppressions via attributes

2021-01-29 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 320068. vsavchenko added a comment. Herald added a subscriber: jdoerfert. Fix failing tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93110/new/ https://reviews.llvm.org/D93110 Files: clang/include/cl

[PATCH] D93110: [analyzer] Implement fine-grained suppressions via attributes

2021-01-29 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2396 + let Args = [VariadicStringArgument<"DiagnosticIdentifiers">]; + let Subjects = SubjectList<[Var]>; + let Documentation = [AnalyzerSuppressDocs]; NoQ wrote: > [evil mode] > W

[PATCH] D93110: [analyzer] Implement fine-grained suppressions via attributes

2021-01-29 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugSuppression.cpp:70 + +class CacheInitializer : public RecursiveASTVisitor { +public: NoQ wrote: > vsavchenko wrote: > > NoQ wrote: > > > A recursive visitor would cause you to visit ne

[PATCH] D93630: [Attr] Apply GNU-style attributes to expression statements

2021-01-29 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 320092. vsavchenko added a comment. Add check for extension Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93630/new/ https://reviews.llvm.org/D93630 Files: clang/include/clang/Basic/Features.def clang/l

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

2021-02-01 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D95307#2532669 , @RedDocMD wrote: > I clearly am taking the wrong approach to this problem. I will need to > reconsider how PointerToMember is actually modelled before solving this > problem I don't know if this is useful

[PATCH] D95846: [analyzer] Updated comments to reflect D85817

2021-02-02 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko accepted this revision. vsavchenko added a comment. This revision is now accepted and ready to land. Thanks for doing that! I have a minor nit-picking comment, it is preferable to use present-tense imperative-style commit messages, i.e. "Update comment...". Repository: rG LLVM Gith

[PATCH] D95877: [analyzer] Fix static_cast on pointer-to-member handling

2021-02-02 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. Thanks for taking your time and getting to the root cause of it! Comment at: clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp:209 +// thing about Clang AST. +std::list BaseList; +for (const auto &I : PathList) `std::li

[PATCH] D95877: [analyzer] Fix static_cast on pointer-to-member handling

2021-02-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. Also, all these `PathRange` and `PathList` don't really reflect what they stand for semantically and talk more about implementation. It's not the problem of this patch, but it doesn't make your logic easier to comprehend. Comment at: clang/lib/Sta

[PATCH] D95877: [analyzer] Fix static_cast on pointer-to-member handling

2021-02-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp:226 +llvm::make_range(BaseList.begin(), BaseList.end()), +[](const auto &I) -> bool { return I.second; }); + RedDocMD wrote: > vsavchenko wrote: > >

[PATCH] D95877: [analyzer] Fix static_cast on pointer-to-member handling

2021-02-05 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D95877#2542578 , @RedDocMD wrote: > @vsavchenko, after some experimentation, I found out that handling > reinterpret_cast will be a real pain. Consider the following: > > struct Base { > int field; > }; > > st

[PATCH] D95877: [analyzer] Fix static_cast on pointer-to-member handling

2021-02-05 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp:210 + "repeated elements"); +BaseSpecSeen.insert(BaseType); } Containers like `map` and `set` usually return a pair for `insert`, where the second

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

2021-02-05 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. Herald added a subscriber: nullptr.cpp. In D92639#2505163 , @ASDenysPetrov wrote: > @vsavchenko > >> F14534708: report-6ea17d.html > > I checked it in IE. It doesn't draw arrows. Investigate

[PATCH] D92928: [analyzer] Highlight arrows for currently selected event

2021-02-05 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:166 + ArrowMap(unsigned Size) : Base(Size, 0) {} + unsigned getTotalNumberOfArrows() const { return at(0); } +}; ASDenysPetrov wrote: > # Whether `size()` is not a

[PATCH] D95877: [analyzer] Fix static_cast on pointer-to-member handling

2021-02-05 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp:180 +bool noRepeatedElements( +const llvm::ImmutableList &BaseSpecList) { nit: functions represent actions, in languages verbs act the same way, so it's recom

[PATCH] D95877: [analyzer] Fix static_cast on pointer-to-member handling

2021-02-05 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp:180 +bool noRepeatedElements( +const llvm::ImmutableList &BaseSpecList) { RedDocMD wrote: > vsavchenko wrote: > > nit: functions represent actions, in language

[PATCH] D95877: [analyzer] Fix static_cast on pointer-to-member handling

2021-02-08 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D95877#2547303 , @RedDocMD wrote: > @vsavchenko are there some more changes you want done? I think we had a bit of misunderstanding about the test. It still should pass (we can't have broken tests, it will disrupt CI bots)

[PATCH] D95877: [analyzer] Fix static_cast on pointer-to-member handling

2021-02-08 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D95877#2548021 , @RedDocMD wrote: > In D95877#2547949 , @vsavchenko > wrote: > >> I think we had a bit of misunderstanding about the test. It still should >> pass (we can't have bro

[PATCH] D96268: [-Wcompletion-handler] Support checks with builtins

2021-02-08 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko created this revision. vsavchenko added a reviewer: NoQ. Herald added a subscriber: Charusso. vsavchenko requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. It is very common to check callbacks and completion handlers for null. This

[PATCH] D96268: [-Wcompletion-handler] Support checks with builtins

2021-02-08 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/lib/Analysis/CalledOnceCheck.cpp:343 +case Builtin::BI__builtin_expect_with_probability: { + assert(CE->getNumArgs() >= 2); + NoQ wrote: > Otherwise it's a compile error and analysis based warnings aren

[PATCH] D96268: [-Wcompletion-handler] Support checks with builtins

2021-02-09 Thread Valeriy Savchenko via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd1522d349f4d: [-Wcompletion-handler] Support checks with builtins (authored by vsavchenko). Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D95877: [analyzer] Fix static_cast on pointer-to-member handling

2021-02-09 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko accepted this revision. vsavchenko added a comment. This revision is now accepted and ready to land. Great job! Thanks for dealing with all my nit-picking! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95877/new/ https://reviews.llvm.or

[PATCH] D93630: [Attr] Apply GNU-style attributes to expression statements

2021-02-10 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D93630#2554186 , @aaron.ballman wrote: > In D93630#2506604 , @aaron.ballman > wrote: > >> In D93630#2504758 , @vsavchenko >> wrote: >> >>> I

[PATCH] D90691: [analyzer] Add new checker for unchecked return value.

2021-02-11 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. Great job! I'd like to see more tests with the following cases: - more expressions with function calls - ternary expression, arithmetic, array literals, initializer lists, etc. - using/not using it in `if` statement with initializer - using/not using it in different p

[PATCH] D93630: [Attr] Apply GNU-style attributes to expression statements

2021-02-11 Thread Valeriy Savchenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG81a970772384: [Attr] Apply GNU-style attributes to expression statements (authored by vsavchenko). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93630/new/

[PATCH] D93110: [analyzer] Implement fine-grained suppressions via attributes

2021-02-12 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D93110#2534690 , @aaron.ballman wrote: > In D93110#2529917 , @NoQ wrote: > >>> What I want to avoid is having a static analyzer-specific suppression >>> attribute, and a different on

[PATCH] D96611: [analyzer][tests] Fix issue comparison script

2021-02-12 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko created this revision. vsavchenko added a reviewer: NoQ. Herald added subscribers: steakhal, ASDenysPetrov, Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. vsavchenko requested review of this revision. Herald added a project:

[PATCH] D96611: [analyzer][tests] Fix issue comparison script

2021-02-13 Thread Valeriy Savchenko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG94a1a5d25f55: [analyzer][tests] Fix issue comparison script (authored by vsavchenko). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96611/new/ https://revie

[PATCH] D85817: [analyzer] Fix crash with pointer to members values

2021-02-15 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D85817#2560570 , @RedDocMD wrote: > @vsavchenko, why did you chose NamedDecl instead of ValueDecl to account for > the indirect fields? I couldn't quite follow it from the discussion above. Looking at it now, I also don't k

[PATCH] D95877: [analyzer] Fix static_cast on pointer-to-member handling

2021-02-15 Thread Valeriy Savchenko via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG21daada95079: [analyzer] Fix static_cast on pointer-to-member handling (authored by RedDocMD, committed by vsavchenko). Repository: rG LLVM Github

[PATCH] D95846: [analyzer] Updated comments to reflect D85817

2021-02-15 Thread Valeriy Savchenko via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf8d3f47e1fd0: [analyzer] Updated comments to reflect D85817 (authored by RedDocMD, committed by vsavchenko). Changed prior to commit: https://revi

[PATCH] D95877: [analyzer] Fix static_cast on pointer-to-member handling

2021-02-15 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D95877#2563383 , @davezarzycki wrote: > The test seems to be broken on Fedora 33 (x86-64 with clang-11): > > XPASS: Clang :: Analysis/reinterpret-cast-pointer-to-member.cpp (6531 of > 74360) > TEST

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

2021-02-17 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D86465#2569403 , @ASDenysPetrov wrote: > @vsavchenko > Hi, I actually want this patch goes developing and be loaded. It's really the > worth one. Is it abandoned? Hi, nope. Just got postponed, I'll address the review com

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

2021-02-19 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:433-434 +if (To == MAX) { + Result.insert(Result.end(), What.begin(), What.end()); + return makePersistent(std::move(Result)); +} ASDenysPetro

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 349537. vsavchenko added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103605/new/ https://reviews.llvm.org/D103605 Files: clang/include/clang/StaticAnalyzer/Core/BugRep

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko marked an inline comment as done. vsavchenko added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:231 + template + void addExpressionHandlerToTheTop(Args &&... ConstructorArgs) { +addExpressionHandlerToThe

[PATCH] D103616: [analyzer] Reimplement trackExpressionValue as ExpressionHandler

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko created this revision. vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus, manas, RedDocMD. Herald added subscribers: ASDenysPetrov, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. vsavchenko requested review of this revision.

[PATCH] D103457: [analyzer] Add forwarding `addVisitor` method

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG92d03c20ea71: [analyzer] Add forwarding `addVisitor` method (authored by vsavchenko). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D103618: [analyzer] Change FindLastStoreBRVisitor to use Tracker

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko created this revision. vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus, manas, RedDocMD. Herald added subscribers: ASDenysPetrov, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. vsavchenko requested review of this revision.

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 349548. vsavchenko added a comment. Minor change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103605/new/ https://reviews.llvm.org/D103605 Files: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugRe

[PATCH] D103616: [analyzer] Reimplement trackExpressionValue as ExpressionHandler

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 349549. vsavchenko added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103616/new/ https://reviews.llvm.org/D103616 Files: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Index: c

[PATCH] D103618: [analyzer] Change FindLastStoreBRVisitor to use Tracker

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 349550. vsavchenko added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103618/new/ https://reviews.llvm.org/D103618 Files: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter

[PATCH] D103618: [analyzer] Change FindLastStoreBRVisitor to use Tracker

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. This one is completely wrong though. Visitor can definitely outlive Tracker, unlike handlers. I need to use some reference-counting solution here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103618/new/ https://revi

[PATCH] D103618: [analyzer] Change FindLastStoreBRVisitor to use Tracker

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 349563. vsavchenko added a comment. Fix dangling reference problem Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103618/new/ https://reviews.llvm.org/D103618 Files: clang/include/clang/StaticAnalyzer/Core

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103605#2796425 , @martong wrote: > In D103605#2796141 , @vsavchenko > wrote: > >> In D103605#2796111 , @martong >> wrote: >> >>> Great in

[PATCH] D103624: [analyzer] Hide and rename FindLastStoreBRVisitor

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko created this revision. vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus, manas, RedDocMD. Herald added subscribers: ASDenysPetrov, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. vsavchenko requested review of this revision.

[PATCH] D103628: [analyzer] Turn ReturnVisitor into a tracking visitor

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko created this revision. vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus, manas, RedDocMD. Herald added subscribers: ASDenysPetrov, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. vsavchenko requested review of this revision.

[PATCH] D103630: [analyzer] Refactor trackRValueExpression into ExpressionHandler

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko created this revision. vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus, manas, RedDocMD. Herald added subscribers: ASDenysPetrov, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. vsavchenko requested review of this revision.

[PATCH] D103631: [analyzer] Turn TrackControlDependencyCond into a tracking visitor

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko created this revision. vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus, manas, RedDocMD. Herald added subscribers: ASDenysPetrov, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. vsavchenko requested review of this revision.

[PATCH] D103633: [analyzer] Refactor trackExpressionValue to accept TrackingOptions

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko created this revision. vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus, manas, RedDocMD. Herald added subscribers: ASDenysPetrov, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. vsavchenko requested review of this revision.

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 349650. vsavchenko added a comment. Add forgotten piece of StoreInfo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103605/new/ https://reviews.llvm.org/D103605 Files: clang/include/clang/StaticAnalyzer/Co

[PATCH] D103616: [analyzer] Reimplement trackExpressionValue as ExpressionHandler

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 349652. vsavchenko added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103616/new/ https://reviews.llvm.org/D103616 Files: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Index: c

[PATCH] D103618: [analyzer] Change FindLastStoreBRVisitor to use Tracker

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 349653. vsavchenko added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103618/new/ https://reviews.llvm.org/D103618 Files: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter

[PATCH] D103624: [analyzer] Hide and rename FindLastStoreBRVisitor

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 349654. vsavchenko added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103624/new/ https://reviews.llvm.org/D103624 Files: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter

[PATCH] D103628: [analyzer] Turn ReturnVisitor into a tracking visitor

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 349655. vsavchenko added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103628/new/ https://reviews.llvm.org/D103628 Files: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Index:

[PATCH] D103630: [analyzer] Refactor trackRValueExpression into ExpressionHandler

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 349656. vsavchenko added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103630/new/ https://reviews.llvm.org/D103630 Files: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Index: c

[PATCH] D103631: [analyzer] Turn TrackControlDependencyCond into a tracking visitor

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 349657. vsavchenko added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103631/new/ https://reviews.llvm.org/D103631 Files: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Index:

[PATCH] D103633: [analyzer] Refactor trackExpressionValue to accept TrackingOptions

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 349659. vsavchenko added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103633/new/ https://reviews.llvm.org/D103633 Files: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter

[PATCH] D103633: [analyzer] Refactor trackExpressionValue to accept TrackingOptions

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 349660. vsavchenko added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103633/new/ https://reviews.llvm.org/D103633 Files: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter

[PATCH] D103644: [analyzer] Refactor StoreSiteFinder and extract DefaultStoreHandler

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko created this revision. vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus, manas, RedDocMD. Herald added subscribers: ASDenysPetrov, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. vsavchenko requested review of this revision.

[PATCH] D103440: [WIP][analyzer] Introduce range-based reasoning for addition operator

2021-06-04 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/test/Analysis/constant-folding.c:282 + +if (c >= -20 && d >= -40) { + clang_analyzer_eval((c + d) < -1); // expected-warning{{TRUE}} manas wrote: > vsavchenko wrote: > > Great, it's good to check negati

[PATCH] D103440: [WIP][analyzer] Introduce range-based reasoning for addition operator

2021-06-04 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103440#2797991 , @manas wrote: >> I also would like to see tests where the ranges are not going all the way to >> either INT_MIN or INT_MAX (if we talk about int), but overflow still might >> happen, and cases where overf

[PATCH] D103677: [analyzer] Extract ControlDependencyHandler

2021-06-04 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko created this revision. vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus, manas, RedDocMD. Herald added subscribers: ASDenysPetrov, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. vsavchenko requested review of this revision.

[PATCH] D103317: [Analyzer][engine][solver] Simplify complex constraints

2021-06-04 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103317#2794099 , @ASDenysPetrov wrote: >> ! In D103317#2793797 , @vsavchenko >> wrote: > > > >> I replied to you earlier that assignments are not producing constraints. >> The a

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-04 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103605#2798171 , @NoQ wrote: > Ok. Oof. Whoa. That's amazing. > > Let me re-iterate to make sure that my understanding of this patchset is > correct. > > **Chapter 1.** //"A is correlated with B (ρ = 0.56), given C, assumi

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-04 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103605#2799853 , @NoQ wrote: > In D103605#2798581 , @vsavchenko > wrote: > >> In D103605#2798171 , @NoQ wrote: >> >>> - Trackers are attac

<    1   2   3   4   5   6   7   8   9   >