[PATCH] D92474: [analyzer][StdLibraryFunctionsChecker] Add return value constraint to functions with BufferSize

steakhal added a comment. I've double-checked the return values of each touched summary. Everything seems fine to me, besides the two I've highlighted. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1771 +.Case({ReturnValueCondition(Le

[PATCH] D92474: [analyzer][StdLibraryFunctionsChecker] Add return value constraint to functions with BufferSize

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Awesome, thank you. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1771 +.Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)), +

[PATCH] D90157: [analyzer] Rework SValBuilder::evalCast function into maintainable and clear way

steakhal added a comment. In D90157#2361773 , @ASDenysPetrov wrote: > Who can confirm if this is correct or somewhere it needs fixes? Here is a > generated result of `evalCast` from the origin branch(before the patch): > > void foo(int* x, // &SymRegi

[PATCH] D92764: [analyzer][StdLibraryFunctionsChecker] Make close and mmap to accept -1 as fd

steakhal added a comment. LGTM besides my inline comment. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1737 .ArgConstraint( ArgumentCondition(4, WithinRange, Range(0, IntMax; `mmap` should hav

[PATCH] D89987: [analyzer] [NFC] Rename SymbolRef to SymExprRef

steakhal added a comment. @ASDenysPetrov Please grep for the `SymbolRef` and rename the other symbols/comments as well, especially the compound names. In D89987#2348935 , @OikawaKirie wrote: > Since `SymbolRef` is just a `const SymExpr *` in the current

[PATCH] D89987: [analyzer] [NFC] Rename SymbolRef to SymExprRef

steakhal added a comment. In D89987#2349959 , @ASDenysPetrov wrote: > @OikawaKirie > >> Different from ProgramStateRef which is an alias to IntrusiveRefCntPtr, or >> StoreRef which is a wrapper object, an alias to a const SymExpr * makes no >> sense to

[PATCH] D90157: [analyzer] Rework SValBuilder::evalCast function into maintainable and clear way

steakhal added a comment. In D90157#2356170 , @martong wrote: >> Now I'm going to make unittests for SValBuilder::evalCast. Let me ask you to >> offer me some examples of casts to check. As many cases we can collect as >> robust the test would be. E.g. >

[PATCH] D74735: [analyzer] Add support for CXXInheritedCtorInitExpr.

steakhal added inline comments. Herald added a subscriber: ASDenysPetrov. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:916-918 + virtual const CXXInheritedCtorInitExpr *getOriginExpr() const { +return cast(AnyFunctionCall::getOriginExpr());

[PATCH] D102280: [analyzer] Engine: fix crash with SEH __leave keyword

steakhal added reviewers: NoQ, vsavchenko, steakhal. steakhal added a comment. Please, add these reviewers for your upcoming [analyzer] patches. Inline a couple of nits. Nice to see some fixes for visual c++ stuff. Comment at: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp:352

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

steakhal added a comment. By checking the line coverage of the `LoopUnrolling.cpp` test file, looks like all lines are covered you touched. There are only two return statements uncovered though: L200, L251. We should consider extending this test file to cover them as well in a follow-up patch.

[PATCH] D102240: [analyzer][solver] Prevent use of a null state

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. In D102240#2756967 , @vsavchenko wrote: > I couldn't transform `c == b` into a condition, so it crashes. Well, that's interesting. It might worth

[PATCH] D102280: [analyzer] Engine: fix crash with SEH __leave keyword

steakhal added a comment. Do you have any good (mature, big enough) open-source projects for these msvc constructs? Comment at: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp:352 + case Stmt::SEHLeaveStmtClass: case Stmt::ContinueStmtClass: AbbasSa

[PATCH] D102280: [analyzer] Engine: fix crash with SEH __leave keyword

steakhal added a comment. In D102280#2759480 , @AbbasSabra wrote: > In D102280#2759167 , @steakhal > wrote: > >> Do you have any good (mature, big enough) open-source projects for these >> msvc constructs? > >

[PATCH] D102280: [analyzer] Engine: fix crash with SEH __leave keyword

steakhal added a comment. In D102280#2760402 , @AbbasSabra wrote: > Is it a requirement for you to build them on Linux? Maybe you can try to > build them on windows with !!clang-cl!!? It is a hard requirement. I'm gonna think more about this in the far

[PATCH] D102280: [analyzer] Engine: fix crash with SEH __leave keyword

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. I think it's good to go. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102280/new/ https://reviews.llvm.org/D102280

[PATCH] D102640: [ASTimporter] Remove decl from lookup only if it has decl context

steakhal created this revision. steakhal added reviewers: teemperor, shafik, balazske, a.sidorin, martong. Herald added subscribers: whisperity, rnkovacs. steakhal requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. In the case of `TypedefDecl`

[PATCH] D102280: [analyzer] Engine: fix crash with SEH __leave keyword

This revision was automatically updated to reflect the committed changes. Closed by commit rGebcf030efc5e: [analyzer] Engine: fix crash with SEH __leave keyword (authored by AbbasSabra, committed by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D102643: [analyzer][Z3][NFC] Use GTEST_SKIP instead of hacks

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

[PATCH] D102643: [analyzer][Z3][NFC] Use GTEST_SKIP instead of hacks

steakhal updated this revision to Diff 346061. steakhal added a comment. now it should build CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102643/new/ https://reviews.llvm.org/D102643 Files: clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp Index: clang/unitte

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

steakhal added a reviewer: balazske. steakhal added a comment. I don't really like having multiple files with the same name. And the importer TU should be simple to be simply `cat`-ed into a temporal file. At that point, you could put the importee's content into this file. It would result in a si

[PATCH] D102640: [ASTimporter] Remove decl from lookup only if it has decl context

This revision was automatically updated to reflect the committed changes. Closed by commit rG88ee91cd8779: [ASTimporter] Remove decl from lookup only if it has decl context (authored by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102640/

[PATCH] D102640: [ASTimporter] Remove decl from lookup only if it has decl context

steakhal added a comment. In D102640#2764759 , @shafik wrote: > This is a good catch, thank you for fixing this! Thanks for the quick review! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102640/new/ http

[PATCH] D102643: [analyzer][Z3][NFC] Use GTEST_SKIP instead of hacks

steakhal updated this revision to Diff 346382. steakhal added a comment. Introduce the test fixture, and use its `setUp()` method for implementing this `GTEST_SKIP` stuff. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102643/new/ https://reviews.l

[PATCH] D102149: [analyzer][ctu] Allow loading invocation list from a compilation database automatically detected from the ctu-dir

steakhal added a comment. There are quite a few places where the extdef mappings should be updated. For discovering them I suggest you asserting the new file format (only for detecting them!). This way if you miss one, it wouldn't silently 'work' somehow, but raise your attention. There are a f

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

steakhal added a comment. There are quite a few places where the extdef mappings should be updated. For discovering them I suggest you asserting the new file format (only for detecting them!). This way if you miss one, it wouldn't silently 'work' somehow, but raise your attention. There are a f

[PATCH] D102835: [analyzer] Correctly propagate ConstructionContextLayer thru ParenExpr

steakhal added a comment. Looks good to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102835/new/ https://reviews.llvm.org/D102835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe

[PATCH] D102643: [analyzer][Z3][NFC] Use GTEST_SKIP instead of hacks

steakhal updated this revision to Diff 346938. steakhal added a comment. Herald added a subscriber: manas. Nothing changed, I'm just retriggering the pre-merge bots as my parent revision landed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102643/new/ https://reviews.llvm.org/D102643

[PATCH] D102643: [analyzer][Z3][NFC] Use GTEST_SKIP instead of hacks

This revision was automatically updated to reflect the committed changes. Closed by commit rG3febf0b507e6: [analyzer][Z3][NFC] Use GTEST_SKIP instead of hacks (authored by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102643/new/ https://

[PATCH] D102835: [analyzer] Correctly propagate ConstructionContextLayer thru ParenExpr

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. In D102835#2773818 , @tomasz-kaminski-sonarsource wrote: > I do not have commit rights. If there are no additional changes needed, would > it be

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

steakhal added a comment. I think It would be also beneficial to document the semantics. Whenever we say `$x + $y` it's not entirely clear whether we talk about the addition operation in a mathematical sense or we follow C/C++ language semantics. Right now it's mostly mixed within the analyzer.

[PATCH] D102835: [analyzer] Correctly propagate ConstructionContextLayer thru ParenExpr

This revision was automatically updated to reflect the committed changes. Closed by commit rG058f384ae94a: [analyzer] Correctly propagate ConstructionContextLayer thru ParenExpr (authored by tomasz-kaminski-sonarsource, committed by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

steakhal added a comment. Could you please reupload your diff to retrigger the bots? I'd like to make sure that everything passes, even the tests that do not belong to you. Unfortunately, I don't know any better way of retriggering the pre-merge checks - I don't have permission to manually retri

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

steakhal accepted this revision. steakhal added a comment. It seems like everything passes. Yeey, good job! Shall I commit this tomorrow on your behalf? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101763/new/ https://reviews.llvm.org/D101763 __

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

This revision was automatically updated to reflect the committed changes. Closed by commit rGdb8af0f21dc9: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand… (authored by OikawaKirie, committed by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

steakhal added a comment. I've decided to land this without the test. Thank you for your contribution. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101763/new/ https://reviews.llvm.org/D101763 ___ cfe-c

[PATCH] D103511: [clang-tidy] Special member functions check should allow sole dtors by default

steakhal created this revision. steakhal added reviewers: aaron.ballman, njames93, steveire. Herald added subscribers: martong, kbarton, xazax.hun, whisperity, nemanjai. steakhal requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. T

[PATCH] D103511: [clang-tidy] Special member functions check should allow sole dtors by default

steakhal added a comment. In D103511#2793291 , @aaron.ballman wrote: > I think the C++ Core Guideline wording is... confusing. The rule title is > `C.21: If you define or =delete any copy, move, or destructor function, > define or =delete them all`, wh

[PATCH] D93222: [analyzer] Introduce MacroExpansionContext to libAnalysis

steakhal updated this revision to Diff 318480. steakhal marked 2 inline comments as done. steakhal retitled this revision from "[RFC][analyzer] Introduce MacroExpansionContext to libAnalysis" to "[analyzer] Introduce MacroExpansionContext to libAnalysis". steakhal edited the summary of this revis

[PATCH] D93223: [analyzer] Create MacroExpansionContext member in AnalysisConsumer

steakhal updated this revision to Diff 318482. steakhal retitled this revision from "[RFC][analyzer] Create MacroExpansionContext member in AnalysisConsumer and pass down to the diagnostics consumers" to "[analyzer] Create MacroExpansionContext member in AnalysisConsumer". steakhal edited the su

[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

steakhal added a comment. The patch doesn't seem to affect the reports. It only introduced 3 new reports during the analysis of clang + clang-tidy. The analysis times indeed increased slightly, ~~ +3%. Overall, I think it's a valuable patch, which resolves crashes when the Z3 refutation enabled.

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

steakhal added a comment. Looks fine to me. Thank you for addressing this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95307/new/ https://reviews.llvm.org/D95307 ___ cfe-commits mailing list cfe-commit

[PATCH] D90157: [analyzer] Rework SValBuilder::evalCast function into maintainable and clear way

steakhal added a comment. What you are basically doing is a visitation on the kind of the `SVal`. Why don't you use the `SValVisitor` instead? That way you could focus on the leaf nodes in the hierarchy, leaving you an even cleaner solution. class CastEvaluator : public SValVisitor { public:

[PATCH] D90157: [analyzer] Rework SValBuilder::evalCast function into maintainable and clear way

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:647-650 + // Array to pointer. + if (isa(OriginalTy)) +if (CastTy->isPointerType() || CastTy->isReferenceType()) return UnknownVal(); ASDenysPetrov wrote: > ste

[PATCH] D90157: [analyzer] Rework SValBuilder::evalCast function into maintainable and clear way

steakhal added a comment. In D90157#2519800 , @ASDenysPetrov wrote: > Updated. Naming fixes. Member access changes. Looks great! IMO we are sort-of late to commit this into clang-12 with confidence. We should land it after we tag that release. What do y

[PATCH] D93222: [analyzer] Introduce MacroExpansionContext to libAnalysis

steakhal updated this revision to Diff 319011. steakhal added a comment. Ignore `_Pragma` macro expansions, also ignore those during lexing. Added a unit-test to demonstrate that we don't crash and these expansions are not recorded. Fix typo in `MacroExpansionContext.cpp` header comment. CHANG

[PATCH] D95860: [clang][Frontend] Fix a crash in DiagnosticRenderer.

steakhal added a comment. Please, also add a regression test for Haoxin Tu's reproducer. // RUN: %clang %s // expected warning volatile long a ( a .~b Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95860/new/ https://reviews.llvm.org/D

[PATCH] D95860: [clang][Frontend] Fix a crash in DiagnosticRenderer.

steakhal added a comment. In D95860#2536721 , @balazske wrote: > In D95860#2536609 , @steakhal wrote: > >> Please, also add a regression test for Haoxin Tu's reproducer. >> >> // RUN: %clang %s >> >> // expec

[PATCH] D95860: [clang][Frontend] Fix a crash in DiagnosticRenderer.

steakhal added a comment. In D95860#2538802 , @balazske wrote: > Existing test moved to Frontend, added new test. Awesome, thanks! On the functional side of things, I'm not sure if we should skip or not. But simply skipping those seems like you are treat

[PATCH] D93787: [analyzer] Fix crash caused by accessing empty map

steakhal added a comment. In D93787#2539111 , @vabridgers wrote: > Abandoning this change request in favor of @steakhal 's more comprehensive > change @ https://reviews.llvm.org/D93222 Let me know if that patch-stack resolves your crash. I don't mind e

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

steakhal added a comment. In D86295#2519851 , @ASDenysPetrov wrote: > What about this change? Did you make more measurements? IMO it needs more justification and measurement to land it. If my measurement was correct then it would decrease the readabilit

[PATCH] D95860: [clang][Frontend] Fix a crash in DiagnosticRenderer.

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. In D95860#2539453 , @balazske wrote: > Probably it is not worth to find a better solution. In the case of > CloneChecker the range starts and ends

[PATCH] D95799: [analyzer] Symbolicate float values with integral casting

steakhal added a comment. void clang_analyzer_dumpi(int); void clang_analyzer_dumpf(float); void clang_analyzer_dumpip(int*); void clang_analyzer_dumpfp(float*); void SymbolCast_of_float_type_aux(int *p) { *p += 1; clang_analyzer_dumpi(*p); // (Previously): Unknown -> (Now):

[PATCH] D96090: [analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::evalCast

steakhal added a comment. Please, consider removing D95799 from the parent revisions if that is not a hard requirement of this patch. It would be awesome to land your patches on dealing with cast problems in the close future. Dealing with floating-point values i

[PATCH] D109608: [clang][ASTImporter] Generic attribute import handling (first step).

steakhal added a comment. I like the adapter layer idea. However, I agree with @martong that it should be more 'abstract' ~ terse. It's remarkable how compact the test is. Good job. Comment at: clang/lib/AST/ASTImporter.cpp:8417-8418 +template struct AttrArgImporter { + Attr

[PATCH] D109608: [clang][ASTImporter] Generic attribute import handling (first step).

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. I think I'm convinced. Thanks. Comment at: clang/unittests/AST/ASTImporterTest.cpp:6414 +// Verify that dump does not crash because invalid data. +ToD->dump(); +

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

steakhal added a comment. In D86295#3014990 , @martong wrote: > @steakhal Since then we have our fancy csa-testbench based jenkins job(s) to > do measurement even on huge projects. Do you think it would make sense to > give it another measure with that?

[PATCH] D110052: [clang][AST] Add support for ShuffleVectorExpr to ASTImporter

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG66d9d1012b03: [clang][AST] Add support for ShuffleVectorExpr to ASTImporter (authored by steakhal). Herald added a project: clang. Herald added a sub

[PATCH] D110357: [Analyzer] Extend ConstraintAssignor to handle remainder op

steakhal added a comment. Great work! Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1591-1595 template LLVM_NODISCARD static ProgramStateRef - assign(ProgramStateRef State, SValBuilder &Builder, RangeSet::Factory &F, - ClassOrSymbol CoS, R

[PATCH] D110387: [Analyzer][NFC] Move RangeConstraintManager's def before ConstraintAssignor's def

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. It seems clean to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110387/new/ https://reviews.llvm.org/D110387 __

[PATCH] D110528: [clang][ASTImporter] Add import of thread safety attributes.

steakhal added a comment. I appreciate the effort in working on the attribute imports. Personally, I don't really like this change. IMO `AttrImporter::createImportedAttr()` should do the magic, and apply the necessary glue for the import. Or we could even create a dedicated function, which woul

[PATCH] D110357: [Analyzer] Extend ConstraintAssignor to handle remainder op

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1591-1595 template LLVM_NODISCARD static ProgramStateRef - assign(ProgramStateRef State, SValBuilder &Builder, RangeSet::Factory &F, - ClassOrSymbol CoS, RangeSet

[PATCH] D110357: [Analyzer] Extend ConstraintAssignor to handle remainder op

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1609-1612 + const SymExpr *LHS = Sym->getLHS(); + const llvm::APSInt &Zero = + Builder.getBasicValueFactory().getValue(0, LHS->getType()); + State = RCM->a

[PATCH] D110625: [analyzer] canonicalize special case of structure/pointer deref

steakhal added a comment. nits Comment at: clang/test/Analysis/ptr-arith.c:336 + int v; + char y; +}; It's probably unnecessary. Comment at: clang/test/Analysis/ptr-arith.c:339 + +void clang_analyzer_dump(int); + I would pr

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

steakhal added a comment. This patch seems pretty straightforward, good job @manas, and for the folks giving review comments. Aside from polishing the tests, I think it's good to go. Comment at: clang/test/Analysis/constant-folding.c:470 + +void testEqualityRules(unsigned int

[PATCH] D109269: [NFC] Cleanup the overload of ASTImporter::import()

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe5e0e00831ba: [NFC] Cleanup the overload of ASTImporter::import() (authored by steakhal). Herald added a project: clang. Herald added a subscriber: c

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

steakhal added a comment. Okay, I don't see any problems besides this typo. BTW do you plan to implement other rules besides this in the future? E.g. we currently miss this: 'x < y' is `true` if `max(x) < min(y)`; and `false` if `min(x) >= max(y)` 'x > y', 'x <= y', etc. in a similar way.

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

steakhal accepted this revision. steakhal added a comment. Good work. Land it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106102/new/ https://reviews.llvm.org/D106102 ___ cfe-commits mailing list cfe-

[PATCH] D110625: [analyzer] canonicalize special case of structure/pointer deref

steakhal added a comment. 'using' is the same as 'typedef' AFAIK. So, you could simply use only typedefs and implement the test in the c test file. Seeing all the tests close together would aid readability IMO. WDYT Denys? Btw does the SVval::getType return a canonical type in all cases? Reposi

[PATCH] D110810: [clang][ASTImporter] Simplify code of attribute import [NFC].

steakhal added a subscriber: aaron.ballman. steakhal added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:8530-8531 + // Get the result of the previous import attempt (can be used only once). + llvm::Expected getResult() { +if (Err) If it can

[PATCH] D110395: [clang][ASTImporter] Import InheritedConstructor and ConstructorUsingShadowDecl.

steakhal added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5880 +AST_MATCHER(CXXConstructorDecl, isInheritingConstructor) { + return Node.isInheritingConstructor(); We need to add doc comments as well. Please address this. Repo

[PATCH] D110911: [analyzer][NFC] Add RangeSet::dump

steakhal added a comment. Please mark both of them `LLVM_DUMP_METHOD`s. This way they will be stripped from release builds according to their documentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110911/new/ https://reviews.llvm.org/D110911

[PATCH] D110625: [analyzer] canonicalize special case of structure/pointer deref

steakhal added a comment. In D110625#3035843 , @ASDenysPetrov wrote: > In D110625#3035616 , @steakhal > wrote: > >> WDYT Denys? Btw does the SVval::getType return a canonical type in all cases? > > `SVal::getTyp

[PATCH] D110913: [analyzer][solver] Handle simplification to ConcreteInt

steakhal added a comment. The patch seems reasonable, but I will need slightly more time to digest it. I'll get back to this. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:390-396 /// Try to simplify a given symbolic expression's

[PATCH] D110625: [analyzer] canonicalize special case of structure/pointer deref

steakhal added a comment. In D110625#3035974 , @ASDenysPetrov wrote: > In D110625#3035929 , @steakhal > wrote: > >> I thought that `SVal::getType` should return an already canonical >> `QualType`. If it doesn't

[PATCH] D110810: [clang][ASTImporter] Simplify code of attribute import [NFC].

steakhal added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:8530-8531 + // Get the result of the previous import attempt (can be used only once). + llvm::Expected getResult() { +if (Err) aaron.ballman wrote: > steakhal wrote: > > If it can b

[PATCH] D107312: [analyzer] Fix deprecated plistlib functions

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. I'm not using this script. I'm assuming you run it and verified that it works. Thanks for cleaning this up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D110927: [analyzer] Access stored value of a constant array through a pointer to another type

steakhal added a subscriber: shafik. steakhal added a comment. I'm pretty sure that `int x4 = ((char*)arr)[1];` is supposed to be valid in your summary. I think it's allowed by the standard to access any valid object via a `char*` - according to the strict aliasing rule. @shafik WDYT? Reposito

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

steakhal added a comment. In D106102#3036220 , @manas wrote: > I do not have landing rights. Please add your name and email on whom behalf I should commit this patch. Mine is `Balazs Benics` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

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

steakhal added a comment. 🛬 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106102/new/ https://reviews.llvm.org/D106102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-b

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

steakhal added a comment. It seems like it doesn't build with GCC 8.3.0 on a Debian system. Could you investigate? llvm-project/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1029:13: error: explicit specialization in non-namespace scope 'class {anonymous}::SymbolicRangeInferrer'

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

steakhal added a comment. I'm gonna get back to this on Monday. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1699-1701 + ProgramStateRef OldState; + do { +OldState = State; IMO we should have a `llvm::Statistic` here, tracking the

[PATCH] D107312: [analyzer] Fix deprecated plistlib functions

This revision was automatically updated to reflect the committed changes. Closed by commit rGa3d0b5805e5f: [analyzer] Fix deprecated plistlib functions (authored by manas, committed by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107312/n

[PATCH] D110810: [clang][ASTImporter] Simplify code of attribute import [NFC].

steakhal accepted this revision. steakhal added a comment. I think it looks great. Thank you for addressing this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110810/new/ https://reviews.llvm.org/D110810 __

[PATCH] D89651: [clang-tidy] Add bugprone-suspicious-memory-comparison check

steakhal added a comment. Herald added a subscriber: nullptr.cpp. I really love this. I'm gonna have a look at the blocking patch. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison-32bits.cpp:2 +// RUN: %check_clang_tidy %s bugprone-s

[PATCH] D89649: Fix __has_unique_object_representations with no_unique_address

steakhal added a comment. It looks great. Comment at: clang/lib/AST/ASTContext.cpp:2582-2584 +int64_t BitfieldSize = Field->getBitWidthValue(Context); +if (BitfieldSize > FieldSizeInBits) + return llvm::None; Why do you return `None` here? ==

[PATCH] D93222: [analyzer] Introduce MacroExpansionContext to libAnalysis

steakhal marked 4 inline comments as done. steakhal added a comment. Thank you @Szelethus for taking the time to review this. This time I marked the inline comments done where it was applicable :) I'm gonna investigate some of your comments and if everything goes well I'm planning to commit this

[PATCH] D93223: [analyzer] Create MacroExpansionContext member in AnalysisConsumer

steakhal added a comment. In D93223#2553053 , @Szelethus wrote: > My no1. thought is that I wish the new functionality you're implementing was > in the interface of the `Preprocessor`. I found, and still find it so hard to > believe that you couldn't jus

[PATCH] D93224: [analyzer] Use the MacroExpansionContext for macro expansions in plists

steakhal marked an inline comment as done. steakhal added a comment. Yey, thanks @Szelethus. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93224/new/ https://reviews.llvm.org/D93224 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D93223: [analyzer] Create MacroExpansionContext member in AnalysisConsumer

steakhal updated this revision to Diff 323024. steakhal added a comment. Nothing changed. Rebased on top of 2407eb08a5748bc2613e95fa449fc1cae6f4ff8f . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93223/new/ https://r

[PATCH] D94673: [analyzer][CTU] API for CTU macro expansions

steakhal updated this revision to Diff 323025. steakhal added a comment. Actually, somehow I messed up something previously. Now, the diff contains the latest version. Sorry for the inconvenience! Rebased on top of 2407eb08a5748bc2613e95fa449fc1cae6f4ff8f

[PATCH] D96090: [analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::evalCast

steakhal added a comment. In D96090#2551664 , @ASDenysPetrov wrote: > Updated. Unlinked parent revision D95799 > from the stack. Made corresponding changes in this patch. > @steakhal I did it. The changes should be safer

[PATCH] D96586: [analyzer][CTU][NFC] Add an extra regression test

steakhal created this revision. steakhal added reviewers: martong, balazske, Szelethus, NoQ, vsavchenko, gamesh411. Herald added subscribers: ASDenysPetrov, Charusso, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, whisperity. steakhal requested r

[PATCH] D96163: [analyzer] Add 12.0.0. release notes

steakhal added a comment. I'm ok with this on my part. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96163/new/ https://reviews.llvm.org/D96163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/lis

[PATCH] D96090: [analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::evalCast

steakhal added a comment. @ASDenysPetrov Could you please rebase this? `git apply` does not seem to apply this patch on top of `llvm/main`. If it depends on any parent revisions please document them as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96090/new/ https://reviews.llvm.o

[PATCH] D87146: [analyzer] Implement shared semantics checks for XNU functions in PthreadLockChecker

steakhal added a comment. Could you please give a few links to some documentation about the functions you are trying to model? Is https://developer.apple.com/library/archive/documentation/Darwin/Conceptual/KernelProgramming/synchronization/synchronization.html a legitimate resource for this? I

[PATCH] D87519: [analyzer][Liveness][NFC] Enqueue the CFGBlocks post-order

steakhal added a comment. What about analyzing a Sema translation unit? That should be beefy enough to have a longer runtime. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87519/new/ https://reviews.llvm.org/D87519 ___

[PATCH] D96090: [analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::evalCast

steakhal added a comment. This patch preserves all previous reports as expected. You can check it by yourself at https://codechecker-demo.eastus.cloudapp.azure.com/Default/runs?run=D96090&items-per-page=50. However, I still have some minor concerns inline. Comment at: clang/

[PATCH] D94673: [analyzer][CTU] API for CTU macro expansions

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:835 + CTU.getMacroExpansionContextForSourceLocation(MacroExpansionLoc)) { +return CTUMacroExpCtx->getExpandedText(MacroExpansionLoc); } balazske wrote:

[PATCH] D96090: [analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::evalCast

steakhal added a comment. In D96090#2572069 , @ASDenysPetrov wrote: > In D96090#2568431 , @steakhal wrote: > >> Could you please rebase this? > > I'll update and rebase this patch soon. > >> If it depends on any pa

[PATCH] D96905: [libclang] Add missing fixed point literal CursorKind to cindex.py

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. LGTM. In D96905#2574322 , @vabridgers wrote: > Polite ping. This is a small and simple change, my only doubt is the test > coverage. But frankly I

<    1   2   3   4   5   6   7   8   9   10   >