[PATCH] D100955: [-Wcalled-once] Do not run analysis on Obj-C++

vsavchenko added a comment. In D100955#4054569 , @thakis wrote: > Can you say a few words on _why_ this is emitted in only Objective-C? What's > missing for Objective-C++? I was surprised that this warning fired in a .m > file but not in a .mm file. Te

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

vsavchenko added a comment. @aaron.ballman a gentle ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93110/new/ https://reviews.llvm.org/D93110 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

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

vsavchenko added a comment. Finally I had a chance to come back to this patch. @aaron.ballman what do you think about it? I tried to address your notes and implemented both features under one attribute. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

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

vsavchenko updated this revision to Diff 367788. vsavchenko added a comment. Herald added a subscriber: manas. Join 'suppress' and 'analyzer_suppress' attributes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93110/new/ https://reviews.llvm.org/D93

[PATCH] D107366: [analyzer] Adjust JS code of analyzer's HTML report for IE support.

vsavchenko accepted this revision. vsavchenko added a comment. This revision is now accepted and ready to land. Now it looks good! Thanks again! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107366/new/ https://reviews.llvm.org/D107366 ___ c

[PATCH] D107636: [analyzer][solver] Compute adjustment for unsupported symbols as well

vsavchenko added a comment. In D107636#2931302 , @steakhal wrote: > Seems reasonable to me. Let's wait for someone else as well. Sure, NP. > This is a really elegant patch, I should tell! Thanks! I guess my take on this, that this path to the solver j

[PATCH] D107636: [analyzer][solver] Compute adjustment for unsupported symbols as well

vsavchenko created this revision. vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus, ASDenysPetrov, manas, RedDocMD. Herald added subscribers: dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. vsavchenko requested review of this revision.

[PATCH] D107366: [analyzer] Adjust JS code of analyzer's HTML report for IE support.

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:1373 + var array = []; + for (var i = lower; i <= upper; ++i) { + array.push(i); ASDenysPetrov wrote: > NoQ wrote: > > Shouldn't it be `i < upper`? > `[lower, u

[PATCH] D107366: [analyzer] Adjust JS code of analyzer's HTML report for IE support.

vsavchenko added a comment. Now it captures (and makes bold) one extra arrow from the previous note. Correct example attached! F18364580: report-e4b488.html CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107366/new/ https://reviews.llvm.org/D107366 _

[PATCH] D107366: [analyzer] Adjust JS code of analyzer's HTML report for IE support.

vsavchenko requested changes to this revision. vsavchenko added a comment. This revision now requires changes to proceed. Oh, wait! I found a bug! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107366/new/ https://reviews.llvm.org/D107366 ___

[PATCH] D107366: [analyzer] Adjust JS code of analyzer's HTML report for IE support.

vsavchenko accepted this revision. vsavchenko added a comment. In D107366#2926662 , @ASDenysPetrov wrote: > In D107366#2926343 , @NoQ wrote: > >> Works in Firefox on macOS as well! > > Great! > @vsavchenko , any

[PATCH] D107366: [analyzer] Adjust JS code of analyzer's HTML report for IE support.

vsavchenko added a comment. Can you please also attach an HTML file just to verify that it works? Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:1293 +var classListAdd = function(el, theClass) { + if(!el.className.baseVal) +el.className += " " + theClass; --

[PATCH] D106823: [analyzer][solver] Iterate to a fixpoint during symbol simplification with constants

vsavchenko added a comment. Looking great! I have a couple of nit picks and I kind of want to check that it doesn't affect the performance on a different set of projects as well. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2112-2113 + assert(ClsMember

[PATCH] D104647: [analyzer] Support SVal::getType for pointer-to-member values

vsavchenko added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h:85 const NamedDecl *D; + QualType T; llvm::ImmutableList L; NoQ wrote: > What prevents you from obtaining the type from `D`? I guess `N

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

vsavchenko added inline comments. Comment at: clang/utils/analyzer/Dockerfile:15-22 RUN apt-get update && apt-get install -y \ git=1:2.17.1* \ gettext=0.19.8.1* \ python3=3.6.7-1~18.04 \ python3-pip=9.0.1-2.3* \ -cmake=3.20.5* \ -ninja-build=1.8.2-1 +

[PATCH] D107339: [analyzer] Retrieve a character from StringLiteral as an initializer for constant arrays.

vsavchenko added a comment. That looks interesting! Can you please add tests, though? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107339/new/ https://reviews.llvm.org/D107339 ___ cfe-commits mailing li

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG9e02f58780ab: [analyzer] Highlight arrows for currently selected event (authored by vsavchenko). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92928/new/ ht

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG97bcafa28deb: [analyzer] Add control flow arrows to the analyzer's HTML reports (authored by vsavchenko). Repository: rG LLVM Github Monorepo CHA

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

vsavchenko added a comment. In D92928#2913552 , @ASDenysPetrov wrote: > Or, we can find another symbiotic way. You can make changes the way without > painfull part of thinking about IE. And I will prepare the next patch > adjusting it. Thus, revisions w

[PATCH] D107026: [Clang] Add support for attribute 'escape'

vsavchenko accepted this revision. vsavchenko added a comment. This revision is now accepted and ready to land. Awesome, I have nothing to add at this point! Let's still wait for @aaron.ballman to check it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D105821: [analyzer] [WIP] Model destructor for std::unique_ptr

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:381 +const MemRegion *ThisRegion = DC->getCXXThisVal().getAsRegion(); +assert(ThisRegion && "We do not support explicit calls to destructor"); +const auto *InnerPtrVal

[PATCH] D105821: [analyzer] [WIP] Model destructor for std::unique_ptr

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:143 +llvm::ArrayRef ValidNames(STD_PTR_NAMES); +return llvm::is_contained(ValidNames, Name); } And why can't we pass `STD_PTR_NAMES` directly to `llvm:

[PATCH] D107026: [Clang] Add support for attribute 'escape'

vsavchenko added a comment. Great job! It looks good, but I have a couple of minor tweaks. I see that the "applies to pointer arguments only" warning is not tested for `noescape`, but I still find it to be a good practice to write a test with a bunch of cases with attributes applied in wrong p

[PATCH] D106739: [analyzer] Add option to SATest.py for extra checkers

vsavchenko added a comment. Looks good! Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106739/new/ https://reviews.llvm.org/D106739 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:/

[PATCH] D106285: [Analyzer][solver] Fix inconsistent equivalence class data

vsavchenko accepted this revision. vsavchenko added a comment. This revision is now accepted and ready to land. Oh, I didn't accept it? Sorry! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106285/new/ https://reviews.llvm.org/D106285

[PATCH] D106416: [analyzer] Fix build dependency issues for SATest

vsavchenko accepted this revision. vsavchenko added a comment. This revision is now accepted and ready to land. Awesome! Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106416/new/ https://reviews.llvm.org/D106416 __

[PATCH] D106296: [analyzer] Fix for faulty namespace test in SmartPtrModelling

vsavchenko added a comment. Can we please land the fix? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106296/new/ https://reviews.llvm.org/D106296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

vsavchenko added a comment. In D103440#2891915 , @manas wrote: > Here is the proof of correctness of the algorithm using Z3: > https://gist.github.com/weirdsmiley/ad6a9dbf3370e96d29f9e90068931d25 There is a couple of fundamental problems there that you

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

vsavchenko added a comment. I want to say that I really appreciate the effort you put into finding all the workarounds for IE, but it makes adding new features here incredibly painful because IE doesn't seem to support anything. And the majority of developers (on Linux and on MacOS) have liter

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

vsavchenko added a comment. Maybe I'm missing something, but do we really need to care about IE? The last version was released in 2013, and even Microsoft itself stops supporting IE. Why should we care? Is there anyone who uses old deprecated browser that is not maintained? `classList` thin

[PATCH] D106296: [analyzer] Fix for faulty namespace test in SmartPtrModelling

vsavchenko accepted this revision. vsavchenko added a comment. Great, LGTM! But let's wait for @xazax.hun anyways Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106296/new/ https://reviews.llvm.org/D106296 __

[PATCH] D106296: [analyzer] Fix for faulty namespace test in SmartPtrModelling

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:252 +static bool isStdFunctionCall(const CallEvent &Call) { + return Call.getDecl() && Call.getDecl() ->getDeclContext()->isStdNamespace(); +} nit: there's an ex

[PATCH] D106296: [analyzer] Fix for faulty namespace test in SmartPtrModelling

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:252-255 + const auto *Decl = Call.getDecl(); + if (!Decl) +return false; + return Decl->getDeclContext()->isStdNamespace(); Can we use a one-liner that I s

[PATCH] D106296: [analyzer] Fix for faulty namespace test in SmartPtrModelling

vsavchenko added a comment. Also, I tested this fix on a set of open-source projects and I don't see any problems. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106296/new/ https://reviews.llvm.org/D106296

[PATCH] D106285: [Analyzer][solver] Fix inconsistent equivalence class data

vsavchenko added a comment. Ah, I see now! I think we could've put together a somewhat easier test knowing what's wrong, but it's not important at all. Thanks for addressing this issue! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106285/new/ ht

[PATCH] D106136: [Analyzer][solver] Fix equivalence class invariant violation in removeDeadBindings

vsavchenko added a comment. In D106136#2889610 , @martong wrote: > In D106136#2883707 , @vsavchenko > wrote: > >> In D106136#2883100 , @martong >> wrote: >> >>> Thanks f

[PATCH] D106296: [analyzer] Fix for faulty namespace test in SmartPtrModelling

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:254-257 + const auto *Decl = Call.getDecl(); + if (!Decl) +return false; + if (!Decl->getDeclContext()->isStdNamespace()) I think you can have a separate f

[PATCH] D106136: [Analyzer][solver] Fix equivalence class invariant violation in removeDeadBindings

vsavchenko added a comment. In D106136#2883100 , @martong wrote: > Thanks for taking your time to take a look. And I accept your statements. > Nevertheless, let me explain my motivation. > > I thought that a class is trivial if it has only one member. An

[PATCH] D106136: [Analyzer][solver] Fix equivalence class invariant violation in removeDeadBindings

vsavchenko added a comment. Thanks for working on it, but it is a quite large change that I don't get the motivation for (it doesn't even fix the recently found bug). Summary explains what the patch does, but for why it is done, it talks about an invariant that is not specified anywhere in the

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1223-1225 +template <> +RangeSet SymbolicRangeInferrer::VisitBinaryOperator(Range LHS, Range RHS, + QualType T) {

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1908-1912 +// Handle SymbolCast before actual assignment. +std::tie(Sym, NewConstraint) = +modifySymbolAndConstraints(Sym, NewConstraint); +if (!State) +

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1908-1912 +// Handle SymbolCast before actual assignment. +std::tie(Sym, NewConstraint) = +modifySymbolAndConstraints(Sym, NewConstraint); +if (!State) +

[PATCH] D106063: [Analyzer][solver] Remove unused functions

vsavchenko accepted this revision. vsavchenko added a comment. This revision is now accepted and ready to land. Yes, let's do this! Thanks for addressing it! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106063/new/ https://reviews.llvm.org/D10606

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

vsavchenko added a comment. In D103096#2877818 , @ASDenysPetrov wrote: > Made `ignoreCast` non-virtual. > P.S. IMO, this change is not something that can be taken as a pattern, > though. It is already a pattern in other type hierarchies. Virtual funct

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

vsavchenko added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:293-296 +SymbolRef Sym = Operand; +while (isa(Sym)) + Sym = cast(Sym)->Operand; +return Sym; ASDenysPetrov wrote: > vsavchenko wr

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

vsavchenko added a comment. > // 1. `VisitSymbolCast`. > // Get a range for main `reg_$0` - [-2147483648, 2147483647] > // Cast main range to `short` - [-2147483648, 2147483647] -> [-32768, > 32767]. > // Now we get a valid range for further bifurcation - [-32768, 32767]. That's a great

[PATCH] D105693: [analyzer][solver][NFC] Refactor how we detect (dis)equalities

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG60bd8cbc0c84: [analyzer][solver][NFC] Refactor how we detect (dis)equalities (authored by vsavchenko). Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D105692: [analyzer][solver][NFC] Introduce ConstraintAssignor

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf26deb4e6ba7: [analyzer][solver][NFC] Introduce ConstraintAssignor (authored by vsavchenko). Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

vsavchenko added a comment. OK, thanks for putting a summary. I now got a good idea why you need both. At the same time, take a look at D105692 . I'm about to land it and I think it's going to be useful for you. CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D105693: [analyzer][solver][NFC] Refactor how we detect (dis)equalities

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:687 /// A small helper structure representing symbolic equality. /// martong wrote: > This is no longer a `structure`. Good catch! Repository: rG LLVM G

[PATCH] D105693: [analyzer][solver][NFC] Refactor how we detect (dis)equalities

vsavchenko updated this revision to Diff 358341. vsavchenko marked an inline comment as done. vsavchenko added a comment. Fix comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105693/new/ https://reviews.llvm.org/D105693 Files: clang/lib/St

[PATCH] D105692: [analyzer][solver][NFC] Introduce ConstraintAssignor

vsavchenko updated this revision to Diff 358336. vsavchenko added a comment. Fix comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105692/new/ https://reviews.llvm.org/D105692 Files: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1203 +if (!Opts.ShouldSupportSymbolicIntegerCasts) + return VisitSymExpr(Sym); + ASDenysPetrov wrote: > vsavchenko wrote: > > ASDenysPetrov wrote: > >

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

vsavchenko added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:293-296 +SymbolRef Sym = Operand; +while (isa(Sym)) + Sym = cast(Sym)->Operand; +return Sym; ASDenysPetrov wrote: > vsavchenko wr

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

vsavchenko added a comment. I'll allocate some time to get into your summary, but for now here are my concerns about `SymbolRangeInferrer` and `VisitSymbolCast`. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1203 +if (!Opts.ShouldSupportSymbolicInteg

[PATCH] D105421: [analyzer] Handle << operator for std::unique_ptr

vsavchenko accepted this revision. vsavchenko added a comment. In D105421#2873458 , @RedDocMD wrote: > Removed stupid mistakes No need for self deprecation here! Thanks for addressing these! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1af97c9d0b02: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda… (authored by AbbasSabra, committed by vsavchenko). R

[PATCH] D105421: [analyzer] Handle << operator for std::unique_ptr

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:87 +static bool hasStdClassWithName(const CXXRecordDecl *RD, +const ArrayRef &Names) { + if (!RD || !RD->getDeclContext()->isStdNamespace()) -

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

vsavchenko added a comment. In D103096#2867441 , @ASDenysPetrov wrote: > @vsavchenko > >> Why did you write it this way!? > > I want the map contains only valid constraints at any time, so we can easely > get them without traversing with all variants in

[PATCH] D105692: [analyzer][solver][NFC] Introduce ConstraintAssignor

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1388 +//===--===// +//New constraint handler +//===--

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

vsavchenko added a comment. In D103096#2867136 , @ASDenysPetrov wrote: > @vsavchenko > >> I still want to hear a good explanation why is it done this way. Here `c` >> is mapped to `(char)x`, and we have `[-10, 10]` directly associated with it, >> but

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

vsavchenko added a comment. In D103096#2867104 , @ASDenysPetrov wrote: > Generally, with this patch we kinda have several constraints for each cast of > a single symbol. And we shall care for all of that constraints and timely > update them (if possibl

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

vsavchenko added a comment. In D103096#2867021 , @ASDenysPetrov wrote: > In D103096#2866730 , @vsavchenko > wrote: > >> In D103096#2866704 , >> @ASDenysPetrov wrote: >>

[PATCH] D105693: [analyzer][solver][NFC] Refactor how we detect (dis)equalities

vsavchenko created this revision. vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus, ASDenysPetrov, manas, RedDocMD. Herald added subscribers: dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. vsavchenko requested review of this revision.

[PATCH] D105692: [analyzer][solver][NFC] Introduce ConstraintAssignor

vsavchenko created this revision. vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus, ASDenysPetrov, manas, RedDocMD. Herald added subscribers: dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. vsavchenko requested review of this revision.

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

vsavchenko added a comment. In D103096#2866704 , @ASDenysPetrov wrote: > @vsavchenko That's not the question I'm asking. Why do you need to set constraints for other symbolic expressions, when `SymbolicInferrer` can look them up on its own? Which ca

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

vsavchenko added a comment. Can you please explain why you do the same thing in two different ways? Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:293-296 +SymbolRef Sym = Operand; +while (isa(Sym)) + Sym = cast(Sym)->Operand; +

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

vsavchenko accepted this revision. vsavchenko added a comment. This revision is now accepted and ready to land. Great! Thanks for addressing all of the comments! Comment at: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp:185-186 + return FD->getType()->isReferenceType();

[PATCH] D105421: [analyzer] Handle << operator for std::unique_ptr

vsavchenko added a comment. Great, thanks for addressing my comments! I still have a couple of minor suggestions though. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:99-100 + +const SmallVector StdPtrNames = {"shared_ptr", "unique_ptr", +

[PATCH] D105421: [analyzer] Handle << operator for std::unique_ptr

vsavchenko added a comment. Good job, great to see how you are going through the whole list of methods! As for the patch, some tests and COMMENTS will be nice. Also, I think that for a checker that models quite a lot of functions and methods, we need to follow the pattern: if (isMethodA(...

[PATCH] D105447: [analyzer] Allow cmake options to be passed to satest container

vsavchenko accepted this revision. vsavchenko added a comment. This revision is now accepted and ready to land. Awesome, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105447/new/ https://reviews.llvm.org/D105447 ___

[PATCH] D105552: [analyzer][NFC] NoStoreFuncVisitor: Compact parameters for region pretty printing into a class

vsavchenko added a comment. Honestly, I don't really see how this is better. IMO `Printer` is something that prints, it should be everything that it does. It can accept different parameters tweaking how it prints in its constructor, but if it is a region printer, you should give it a region, an

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2801-2802 + // Get a root symbol in case of SymbolCast. + while (isa(Sym)) +Sym = cast(Sym)->getOperand(); + ASDenysPetrov wrote: > vsavchenko wrote: > >

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

vsavchenko added a comment. This is a very complicated patch, I think we'll have to iterate on it quite a lot. Additionally, we have to be sure that this doesn't crash our performance. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1131 +class NominalTy

[PATCH] D105436: [analyzer][solver] Use all sources of constraints

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:908 +// into subexpressions of Sym. +Visit(Sym)); } martong wrote: > vsavchenko wrote: > > martong wrote: > > > Alright. So, this is correct

[PATCH] D105436: [analyzer][solver] Use all sources of constraints

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:908 +// into subexpressions of Sym. +Visit(Sym)); } martong wrote: > Alright. So, this is correct because `Visit` boils down finally to eithe

[PATCH] D105436: [analyzer][solver] Use all sources of constraints

This revision was automatically updated to reflect the committed changes. Closed by commit rG6017cb31bb35: [analyzer][solver] Use all sources of constraints (authored by vsavchenko). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105436/new/ https://

[PATCH] D105447: [analyzer] Allow cmake options to be passed to satest container

vsavchenko added inline comments. Comment at: clang/utils/analyzer/entrypoint.py:12 settings, rest = parse_arguments() +cmake_opts = list(filter(lambda cmd: cmd[:2]=='-D', rest)) if settings.wait: I think we should still use `argparse` for stuff lik

[PATCH] D105436: [analyzer][solver] Use all sources of constraints

vsavchenko added a comment. I compared issues produced by this patch to the issues produced before that on all projects from `clang/utils/analyzer/projects`, and didn't find any difference. Performance measurements also show the we are within the same margins. F17774659: photo_2021-07-05 19.39.

[PATCH] D105436: [analyzer][solver] Use all sources of constraints

vsavchenko created this revision. vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus, ASDenysPetrov, manas, RedDocMD. Herald added subscribers: dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. vsavchenko requested review of this revision.

[PATCH] D105273: [analyzer] Introduce range-based reasoning for subtraction operator

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1485 + if (ResultType.isUnsigned() || + ((LHS.From() >= 0 && RHS.From() < 0) && + (LHS.To() >= 0 && RHS.To() < 0)) || manas wrote: > @vsavchenko one

[PATCH] D105340: [analyzer] Produce SymbolCast symbols for integral types in SValBuilder::evalCast

vsavchenko added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def:318 +ANALYZER_OPTION(bool, ShouldHandleIntegralCastForRanges, +"handle-integral-cast-for-ranges", +"Handle truncations, promotions and convers

[PATCH] D105340: [analyzer] Produce SymbolCast symbols for integral types in SValBuilder::evalCast

vsavchenko added a comment. Also, even though the test is very extensive it is pretty lopsided at the same time. C-style cast is only one case out of the myriad of all explicit and, more importantly, implicit casts. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105340/new/ https://re

[PATCH] D105273: [analyzer] Introduce range-based reasoning for subtraction operator

vsavchenko added a comment. Hey Manas! Great job, you put this together real quick! Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1490 + // the resulting range should be [Min, Max]. + if (ResultType.isUnsigned()) { +return {RangeFactory, ValueFacto

[PATCH] D105340: [analyzer] Produce SymbolCast symbols for integral types in SValBuilder::evalCast

vsavchenko added inline comments. Comment at: clang/test/Analysis/produce-symbolcast.cpp:1 +// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -analyzer-config handle-integral-cast-for-ranges=true -verify %s + ASDenysPetrov wrote: > vsavchenko wro

[PATCH] D105340: [analyzer] Produce SymbolCast symbols for integral types in SValBuilder::evalCast

vsavchenko added inline comments. Comment at: clang/test/Analysis/range_casts.c:125-126 if (index - 1UL == 0L) // Was not reached prior fix. -clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} +// Tempopary regression in scope of implementing integral cas

[PATCH] D105340: [analyzer] Produce SymbolCast symbols for integral types in SValBuilder::evalCast

vsavchenko added a comment. Hey, thanks for starting on splitting into more pieces! Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:96 QualType OriginalTy); + SVal simplifySymbolCast(nonloc::SymbolVal V, QualType CastTy)

[PATCH] D105167: [analyzer] Fix HTML report deduplication.

vsavchenko accepted this revision. vsavchenko added a comment. This revision is now accepted and ready to land. This is incredible! Thanks for addressing it! I've encountered this many times. Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:277 + + std::strings

[PATCH] D104550: [analyzer] Implement getType for SVal

vsavchenko added a comment. In D104550#2849561 , @DavidSpickett wrote: > @vsavchenko One of the added tests is failing on our 32 bit Armv7 Thumb bot: > https://lab.llvm.org/buildbot/#/builders/170/builds/61 > > > /home/tcwg-buildslave/worker/clang-th

[PATCH] D105017: [analyzer] LValueToRValueBitCasts should evaluate to an r-value

vsavchenko accepted this revision. vsavchenko added a comment. Oh, wow, that's awesome! Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105017/new/ https://reviews.llvm.org/D105017 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D105101: [analyzer] Make CheckerManager::hasPathSensitiveCheckers() complete again

vsavchenko accepted this revision. vsavchenko added a comment. This revision is now accepted and ready to land. Great! Thanks for the detailed explanation in your Summary. I agree that there should be a runtime check, but it doesn't seem viable as long as `Checker.h` is organized the way it is o

[PATCH] D104550: [analyzer] Implement getType for SVal

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG159024ce2315: [analyzer] Implement getType for SVal (authored by vsavchenko). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D104647: [analyzer] Support SVal::getType for pointer-to-member values

vsavchenko updated this revision to Diff 355154. vsavchenko added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104647/new/ https://reviews.llvm.org/D104647 Files: clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValu

[PATCH] D104550: [analyzer] Implement getType for SVal

vsavchenko updated this revision to Diff 355152. vsavchenko added a comment. Add one more note to `getType` docstring Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104550/new/ https://reviews.llvm.org/D104550 Files: clang/include/clang/StaticAna

[PATCH] D103967: [Analyzer][solver] Add dump methods for (dis)equality classes.

vsavchenko added a comment. In D103967#2844140 , @thakis wrote: > Thanks. Looks like it flakily fails on mac too every now and then: > http://45.33.8.238/mac/33005/step_7.txt > > Maybe you print something in nondeterministic iteration order? Yes, I thin

[PATCH] D105005: [analyzer][solver][NFC] Simplify function signatures

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8474bb13c327: [analyzer][solver][NFC] Simplify function signatures (authored by vsavchenko). Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D105005: [analyzer][solver][NFC] Simplify function signatures

vsavchenko created this revision. vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus, ASDenysPetrov, manas, RedDocMD. Herald added subscribers: dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. vsavchenko requested review of this revision.

[PATCH] D104716: [analyzer] Fix assertion failure on code with transparent unions

This revision was automatically updated to reflect the committed changes. Closed by commit rGd646157146cc: [analyzer] Fix assertion failure on code with transparent unions (authored by vsavchenko). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104716

[PATCH] D103967: [Analyzer][solver] Add dump methods for (dis)equality classes.

vsavchenko accepted this revision. vsavchenko added a comment. This revision is now accepted and ready to land. Awesome, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103967/new/ https://reviews.llvm.org/D103967 ___

[PATCH] D104917: [Analyzer] Extend exploded-graph-rewriter to support eq and diseq classes

vsavchenko added a comment. LGTM, but I'm not really an expert in `exploded-graph-rewriter`. I think @NoQ should take a look. Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:299 +self.equivalence_classes = ( +GenericMap({i: ", ".join(eq_classes

  1   2   3   4   5   6   7   8   9   >