[PATCH] D125318: [analyzer] Add UnarySymExpr

martong added a comment. Related false positive: https://github.com/llvm/llvm-project/issues/55241 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125318/new/ https://reviews.llvm.org/D125318 ___ cfe-commi

[PATCH] D125318: [analyzer] Add UnarySymExpr

martong marked 9 inline comments as done. martong added a comment. Thanks for the review guys! > It feels like `SymbolCast` is a `UnarySymExpr`, but I like to have a distinct > entity for this. Yes, their implementation is similar, but there are subtle differences. > Anyway. There are a lot mo

[PATCH] D125318: [analyzer] Add UnarySymExpr

martong updated this revision to Diff 428601. martong marked 6 inline comments as done. martong added a comment. - Handle UnarySymExpr in SValExplainer - Handle UnarySymExpr in SymbolExpressor - Handle UnarySymExpr in SimpleSvalBuilder's Simplifier - Better assertions - Handle UO_Not Repository:

[PATCH] D125340: [clang][NFC][AST] rename the ImportError to ASTImportError

martong added a comment. What are the benefits of this renaming? I mean is there a name clash? Do we have another kind of "import" in Clang or in some of the dependent projects, don't we? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125340/new/

[PATCH] D125379: [analyzer][solver] Do not negate unsigned ranges

martong created this revision. martong added reviewers: NoQ, steakhal. Herald added subscribers: manas, ASDenysPetrov, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: Szelethus. Herald added a project:

[PATCH] D125379: [analyzer][solver] Do not negate unsigned ranges

martong abandoned this revision. martong added inline comments. Comment at: clang/test/Analysis/constraint_manager_negate_difference.c:125-130 void negate_unsigned_mid(unsigned m, unsigned n) { if (m - n == UINT_MID) { -clang_analyzer_eval(n - m == UINT_MID); // expected-

[PATCH] D125395: [analyzer][solver] Handle UnarySymExpr in RangeConstraintSolver

martong created this revision. martong added reviewers: steakhal, NoQ. Herald added subscribers: manas, ASDenysPetrov, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: Szelethus. Herald added a project:

[PATCH] D125395: [analyzer][solver] Handle UnarySymExpr in RangeConstraintSolver

martong updated this revision to Diff 428683. martong added a comment. - Add false positive test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125395/new/ https://reviews.llvm.org/D125395 Files: clang/lib/StaticAnalyzer/Core/RangeConstraintManag

[PATCH] D125395: [analyzer][solver] Handle UnarySymExpr in RangeConstraintSolver

martong added a comment. Fixes https://github.com/llvm/llvm-project/issues/55241 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125395/new/ https://reviews.llvm.org/D125395 ___ cfe-commits mailing list cf

[PATCH] D125340: [clang][NFC][AST] rename the ImportError to ASTImportError

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Yeah, okay, this patch makes sense now that I've seen a clash with python's ImportError . I've checked lldb c++ files and `ImportError` is

[PATCH] D124674: [analyzer] Indicate if a parent state is infeasible

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp:46 +ConstraintManager::ProgramStatePair +ConstraintManager::assumeDual(ProgramStateRef State, DefinedSVal Cond) { + ProgramStateRef StTrue = assume(State, Cond, true);

[PATCH] D125360: [analyzer] Add taint to the BoolAssignmentChecker

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125360/new/ https://reviews.llvm.org/D125360

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

martong added a comment. Nice work! Could you pleas add some lit tests that describe an errno related bugreport for a standard lib function? Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:381 + class ErrnoConstraintKind { + public: ---

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

martong marked 3 inline comments as done. martong added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:116 + const bool Foreign = false; // From CTU. + xazax.hun wrote: > martong wrote: > > martong wrote: > > >

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

martong added a comment. The bugreports look promising. However, I think we desperately need a note that describes which function has set the `errno_modeling` state. Below I'd expect the following notes for the highlighted function call. - assuming return value of `mkdir` is in range [0, INT_MA

[PATCH] D125395: [analyzer][solver] Handle UnarySymExpr in RangeConstraintSolver

martong marked 13 inline comments as done. martong added a comment. In D125395#3506854 , @steakhal wrote: > Great content! > I've got a long list of nits though. Nothing personal :D No worries, thank you for being assiduous. Comment a

[PATCH] D125395: [analyzer][solver] Handle UnarySymExpr in RangeConstraintSolver

martong updated this revision to Diff 428932. martong marked 6 inline comments as done. martong added a comment. - Address steakhal review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125395/new/ https://reviews.llvm.org/D125395 Files:

[PATCH] D125463: [analyzer][NFC] Tighten some of the SValBuilder return types

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Seems like it compiles, so LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125463/new/ https://reviews.llvm.org/D125463 _

[PATCH] D125318: [analyzer] Add UnarySymExpr

martong marked an inline comment as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:107 +return makeNonLoc(X.castAs().getSymbol(), UO_Not, + X.getType(Context)); default: steakhal wr

[PATCH] D125318: [analyzer] Add UnarySymExpr

martong updated this revision to Diff 429152. martong marked an inline comment as done. martong added a comment. - Add type parameter to evalMinus and evalComplement - Correct dumpToStream in case of binary sub expressions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D123685: [clang][ASTImporter] Add isNewDecl

martong marked 4 inline comments as done. martong added inline comments. Comment at: clang/include/clang/AST/ASTImporterSharedState.h:83 + + void setNewDecl(Decl *ToD) { NewDecls.insert(ToD); } }; `markAsNewDecl` sounds better, I'll update before commit. Repo

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

martong marked 15 inline comments as done. martong added inline comments. Comment at: clang/include/clang/AST/ASTImporterSharedState.h:83 + + void setNewDecl(Decl *ToD) { NewDecls.insert(ToD); } }; whisperity wrote: > (The naming of this function feels a bit od

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

martong updated this revision to Diff 429172. martong marked 7 inline comments as done. martong added a comment. - Change -mul to -pct - Change default values to ctu-max-nodes-pct = 50 and ctu-max-nodes-min = 1 - Rename isCTUInFirtstPhase to isSecondPhaseCTU and invert the condition - Check th

[PATCH] D125318: [analyzer] Add UnarySymExpr

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:107 +return makeNonLoc(X.castAs().getSymbol(), UO_Not, + X.getType(Context)); default: steakhal wrote: > martong wrote: > > steakhal wrote:

[PATCH] D125318: [analyzer] Add UnarySymExpr

martong updated this revision to Diff 429207. martong added a comment. - Revert "Add type parameter to evalMinus and evalComplement" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125318/new/ https://reviews.llvm.org/D125318 Files: clang/include/

[PATCH] D125395: [analyzer][solver] Handle UnarySymExpr in RangeConstraintSolver

martong updated this revision to Diff 429208. martong added a comment. - Add a test case for casts related crash Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125395/new/ https://reviews.llvm.org/D125395 Files: clang/lib/StaticAnalyzer/Core/Rang

[PATCH] D125318: [analyzer] Add UnarySymExpr

martong marked 2 inline comments as done. martong added inline comments. Comment at: clang/test/Analysis/unary-sym-expr.c:35 +return; + clang_analyzer_eval(-(x + y) == -3); // expected-warning{{TRUE}} +} steakhal wrote: > Does it work if you swap x and y? No

[PATCH] D125532: [analyzer] Introduce clang_analyzer_dumpSvalType introspection function

martong accepted this revision. martong added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp:268 + QualType Ty = C.getSVal(Arg).getType(C.getASTContext()); + reportBug(Ty.getAsString(), C

[PATCH] D125524: [BoundV2] ArrayBoundV2 checks if the extent is tainted

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:208 if (state_exceedsUpperBound && state_withinUpperBound) { - SVal ByteOffset = rawOffset.getByteOffset(); - if (isTainted(state, ByteOffset)) { + if (isTaint

[PATCH] D123685: [clang][ASTImporter] Add isNewDecl

martong updated this revision to Diff 429212. martong added a comment. - setNewDecl -> markAsNewDecl Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123685/new/ https://reviews.llvm.org/D123685 Files: clang/include/clang/AST/ASTImporterSharedState

[PATCH] D125547: [analyzer][solver] Handle UnarySymExpr in SMTConv

martong created this revision. martong added reviewers: steakhal, NoQ, mikhail.ramalho. Herald added subscribers: manas, ASDenysPetrov, gamesh411, dkrupp, donat.nagy, Szelethus, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: Szelethus. Herald added a project:

[PATCH] D125524: [BoundV2] ArrayBoundV2 checks if the extent is tainted

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:208 if (state_exceedsUpperBound && state_withinUpperBound) { - SVal ByteOffset = rawOffset.getByteOffset(); - if (isTainted(state, ByteOffset)) { + if (isTaint

[PATCH] D125547: [analyzer][solver] Handle UnarySymExpr in SMTConv

martong marked 2 inline comments as done. martong added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h:258 + static inline llvm::SMTExprRef fromUnary(llvm::SMTSolverRef &Solver, + ASTContex

[PATCH] D125547: [analyzer][solver] Handle UnarySymExpr in SMTConv

martong updated this revision to Diff 429633. martong marked 2 inline comments as done. martong added a comment. - Use existing fromUnOp - pass nullptr as FromTy Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125547/new/ https://reviews.llvm.org/D12

[PATCH] D125547: [analyzer][solver] Handle UnarySymExpr in SMTConv

martong added a comment. In D125547#3515259 , @steakhal wrote: > Something is messed up with the diff. You refer to `fromUnOp()` but it's not > defined anywhere. No. There is no mess up here, the diff is correct. `fromUnOp` had been implemented way bef

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

martong added a comment. In D125400#3508955 , @balazske wrote: > Function `mkdir` is modeled incorrectly by the checker. According to the man > page it can return 0 or -1 only (-1 is error) but the checker allows > non-negative value at success. So the

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

martong added a comment. In D125400#3516164 , @balazske wrote: > @martong Do you mean that a "describe" function is needed for the return > value constraint of the function and for the errno constraint type? Then a > note tag can contain what the functi

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:435 + assert(Cond); + State = State->assume(*Cond, true); + return errno_modeling::setErrnoValue(State, C.getLocationContext(), balazske wro

[PATCH] D125709: [analyzer][Casting] Support isa, cast, dyn_cast of SVals

martong added a comment. > I'm not sure, shall I add tests? Yes, please. Unit tests for `dyn_cast` and `isa` should be easy. However, I am not sure how to test `cast` for the failure cases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125709/new/

[PATCH] D125706: [analyzer][NFC] Use idiomatic classof instead of isKind

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125706/new/ https://reviews.llvm.org/D125706 _

[PATCH] D125707: [analyzer][NFC] Remove unused friend SVal declarations

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125707/new/ https://reviews.llvm.org/D125707 _

[PATCH] D125708: [analyzer][NFC] Remove unused default SVal constructors

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125708/new/ https://reviews.llvm.org/D125708 _

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

martong marked an inline comment as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:446 +} +const bool BState = State->contains(D); +if (!BState) { // This is the first time we see this foreign function. -

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

martong updated this revision to Diff 430023. martong marked an inline comment as done. martong added a comment. - Change ctuBifurcate to use bool in GDM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123773/new/ https://reviews.llvm.org/D123773 Fi

[PATCH] D123784: [clang][analyzer][ctu] Traverse the ctu CallEnter nodes in reverse order

martong abandoned this revision. martong marked an inline comment as done. martong added a comment. Abandoning because by using a `bool` in `ctuBifurcate`, the CTUWorklist will not have more than one elements during the first phase. Details: https://reviews.llvm.org/D123773?id=426037#inline-1206

[PATCH] D125771: [clang-tidy] Add a useful note about -std=c++11-or-later

martong added inline comments. Comment at: clang-tools-extra/test/clang-tidy/check_clang_tidy.py:31 + -std=c++11-or-later: +This flag will cause multiple runs withing the same check_clang_tidy +execution. Make sure you don't have shared state across these runs. -

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

martong updated this revision to Diff 430047. martong added a comment. - Update ReleaseNotes.rst Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123773/new/ https://reviews.llvm.org/D123773 Files: clang/docs/ReleaseNotes.rst clang/include/clang/

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

martong marked an inline comment as done. martong added inline comments. Comment at: clang/docs/ReleaseNotes.rst:352 + reports are lost compared to single-TU analysis, the lost reports are highly + likely to be false positives. xazax.hun wrote: > I wonder if

[PATCH] D123685: [clang][ASTImporter] Add isNewDecl

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG25ac078a961d: [clang][ASTImporter] Add isNewDecl (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. martong marked an inline comment as done. Closed by commit rG56b9b97c1ef5: [clang][analyzer][ctu] Make CTU a two phase analysis (authored by martong). Changed prior to

[PATCH] D125871: [analyzer] Delete alpha.deadcode.UnreachableCode checker

martong created this revision. martong added reviewers: NoQ, steakhal, Szelethus. Herald added subscribers: manas, ASDenysPetrov, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, mgorny. Herald added a project: All. martong requested revi

[PATCH] D125871: [analyzer] Delete alpha.deadcode.UnreachableCode checker

martong added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:847 -let ParentPackage = DeadCodeAlpha in { - TODO, update the ReleseNotes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D125892: [analyzer] Implement assumeInclusiveRange in terms of assumeInclusiveRangeDual

martong created this revision. martong added reviewers: steakhal, NoQ. Herald added subscribers: manas, ASDenysPetrov, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: Szelethus. Herald added a project:

[PATCH] D125871: [analyzer] Delete alpha.deadcode.UnreachableCode checker

martong marked an inline comment as done. martong added a comment. In D125871#3521967 , @steakhal wrote: > Could you please give a few examples of these FPs for the record? Out of thin air I could come up with the following one below. Seems like `try` i

[PATCH] D125892: [analyzer] Implement assumeInclusiveRange in terms of assumeInclusiveRangeDual

martong added a comment. In D125892#3523296 , @steakhal wrote: > Okay, it took me a while to get what's going on. > Do you have anything in mind to express it by slightly less indirection, > references, templates and well, virtual functions xD Okay, I s

[PATCH] D125892: [analyzer] Implement assumeInclusiveRange in terms of assumeInclusiveRangeDual

martong updated this revision to Diff 430611. martong added a comment. - Split into two patches, this first patch will be just the no-brainer copy-paste from assumeDual implementaton. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125892/new/ https

[PATCH] D125954: [analyzer][NFC] Factor out the copy-paste code repetition of assumeDual and assumeInclusiveRangeDual

martong created this revision. martong added reviewers: NoQ, steakhal. Herald added subscribers: manas, ASDenysPetrov, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: Szelethus. Herald added a project:

[PATCH] D125920: [analyzer][NFC] Remove the unused LocAsInteger::getPersistentLoc()

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM. Are there any other `getPersistentLoc` function definitions in other SVals? Might they be also unused? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D125892: [analyzer] Implement assumeInclusiveRange in terms of assumeInclusiveRangeDual

martong updated this revision to Diff 430637. martong marked an inline comment as done. martong edited the summary of this revision. martong added a comment. - Move SimpleConstraintManager's assume*Internal functions to be protected Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D125892: [analyzer] Implement assumeInclusiveRange in terms of assumeInclusiveRangeDual

martong added a comment. In D125892#3524453 , @steakhal wrote: > Now I see, the summary confused me. > >> This includes the refactoring of the common assumle*Dual logic into the >> function template assumeDualImpl. > > I felt like this is a refactoring ch

[PATCH] D125547: [analyzer][solver] Handle UnarySymExpr in SMTConv

martong added a comment. There are Z3 refutation related assertions on open-source projects once the patch stack is applied. Interestingly it happens in `fromBinOp`. clang-14: ../../git/llvm-project/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h:96: static const llvm::SMTExp

[PATCH] D125986: [clang][ASTImporter] Add support for import of UsingPackDecl.

martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:4861 + ToUsingPack->setLexicalDeclContext(LexicalDC); + LexicalDC->addDeclInternal(ToUsingPack); + Why don't we use `addDeclToContexts`? Comment at: clang/unittests/AST/

[PATCH] D126067: [analyzer] Drop the unused 'analyzer-opt-analyze-nested-blocks' cc1 flag

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. > However, this arises a couple of burning questions. > > Should the cc1 frontend still accept this flag - to keep tools/users passing > this flag directly to cc1 (which is unsupported, unadv

[PATCH] D126067: [analyzer] Drop the unused 'analyzer-opt-analyze-nested-blocks' cc1 flag

martong added a comment. Uhh, ohh, don't forget to reflect this change in the ReleaseNotes.rst, so urers will be notified! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126067/new/ https://reviews.llvm.org/D126067

[PATCH] D125892: [analyzer] Implement assumeInclusiveRange in terms of assumeInclusiveRangeDual

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG32f189b0d9a8: [analyzer] Implement assumeInclusiveRange in terms of assumeInclusiveRangeDual (authored by martong). Repository: rG LLVM Github Mon

[PATCH] D125954: [analyzer][NFC] Factor out the copy-paste code repetition of assumeDual and assumeInclusiveRangeDual

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG96fba640cf58: [analyzer][NFC] Factor out the copy-paste code repetition of assumeDual and… (authored by martong). Repository: rG LLVM Github Monor

[PATCH] D126126: [analyzer][NFC] Inline and simplify nonloc::ConcreteInt functions

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:90 -SVal SimpleSValBuilder::evalMinus(NonLoc val) { - switch (val.getSubKind()) { - case nonloc::ConcreteIntKind: I'd rather keep the `switch` because in D125318 we a

[PATCH] D126127: [analyzer][NFC] Relocate unary transfer functions

martong added a comment. > This is an initial step of removing the SimpleSValBuilder abstraction. The > SValBuilder alone should be enough. Perhaps this should be part of another patch stack? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126127/ne

[PATCH] D126128: [analyzer][NFC] Inline loc::ConcreteInt::evalBinOp

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Looks, good, but it was a struggle to follow if you did the inlining right or not. TBH, someone else should also check it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D126130: [analyzer][NFC] Remove unused SVal::hasConjuredSymbol

martong added a comment. > Remove unused SVal::hasConjuredSymbol The title seems to be off with the changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126130/new/ https://reviews.llvm.org/D126130 __

[PATCH] D126123: [analyzer][NFC] MemRegion::getRegion() never returns null

martong added a comment. Is it documented with `getRegion`? Could we decorate that with `returns-nonnull` https://clang.llvm.org/docs/AttributeReference.html#returns-nonnull ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126123/new/ https://rev

[PATCH] D126198: [analyzer][NFCi] Annotate major nonnull returning functions

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Looks good, with minor revisions. Comment at: clang/include/clang/Analysis/AnalysisDeclContext.h:233 + : Kind(k), Ctx(ctx), Parent(parent), ID(ID) { +assert(Ctx);

[PATCH] D126216: [analyzer][NFC] Remove unused RegionStoreFeatures

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126216/new/ https://reviews.llvm.org/D126216 _

[PATCH] D126215: [analyzer] Deprecate `-analyzer-store region` flag

martong added a comment. > We should support deprecated analyzer flags for at least one release. In this > case I'm planning to drop this flag in clang-17 Should we emit a warning for the user about the deprecation? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D126281: [analyzer] Fix symbol simplification assertion failure

martong created this revision. martong added reviewers: NoQ, steakhal. Herald added subscribers: manas, ASDenysPetrov, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: Szelethus. Herald added a project:

[PATCH] D126281: [analyzer] Fix symbol simplification assertion failure

martong added inline comments. Comment at: clang/test/Analysis/symbol-simplification-assertion.c:15-18 +void clang_analyzer_printState(); +void clang_analyzer_dump(long); +void clang_analyzer_eval(int); + TODO remove. Repository: rG LLVM Github Monorepo CHAN

[PATCH] D126215: [analyzer] Deprecate `-analyzer-store region` flag

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126215/new/ https://reviews.llvm.org/D126215

[PATCH] D126281: [analyzer] Fix symbol simplification assertion failure

martong added a comment. > Point 2) above has negligible performance impact, empirical measurements do > not show any noticeable difference in the run-time. Attaching the measurement results that show the run-time difference is not noticeable, or rather it is within the margin of error. F23176

[PATCH] D126123: [analyzer][NFC] MemRegion::getRegion() never returns null

martong added a comment. In D126123#3531150 , @steakhal wrote: > In D126123#3531112 , @martong wrote: > >> Is it documented with `getRegion`? Could we decorate that with >> `returns-nonnull` >> https://clang.ll

[PATCH] D126281: [analyzer] Fix symbol simplification assertion failure

martong updated this revision to Diff 431635. martong marked an inline comment as done. martong added a comment. - Add clang_analyzer_warnIfReached() to the test and remove unused debug function decls Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D126281: [analyzer] Fix symbol simplification assertion failure

martong added inline comments. Comment at: clang/test/Analysis/symbol-simplification-assertion.c:27 + assert(a + L1 + 1 + b != c); + assert(a == 0); // no-assertion +} steakhal wrote: > Is this statement still reachable? Demonstrate it by using the > `clang_an

[PATCH] D66348: [ASTImporter] Do not look up lambda classes

martong updated this revision to Diff 216378. martong added a comment. - Add tests for default constructible and assignable stateless lambdas Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66348/new/ https://reviews.llvm.org/D66348 Files: clang/l

[PATCH] D66348: [ASTImporter] Do not look up lambda classes

martong added a comment. In D66348#1636564 , @shafik wrote: > I am not enthusiastic about this solution but I need to think about it some > more. > > We can see that p0624r2 > added >

[PATCH] D66538: [AST] AST structural equivalence to work internally with pairs.

martong added inline comments. Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:1586 - if (EquivToD1) -return EquivToD1 == D2->getCanonicalDecl(); balazske wrote: > It looks like that the original code is correct in the decision of structural > equi

[PATCH] D66538: [AST] AST structural equivalence to work internally with pairs.

martong added inline comments. Comment at: clang/unittests/AST/StructuralEquivalenceTest.cpp:1323 +NonEquivalentDecls.end()); + EXPECT_EQ(NonEquivalentDecls.find(std::make_pair(y1, y2)), +NonEquivalentDecls.end()); This should be `EXPECT_

[PATCH] D66336: [ASTImporter] Add development internals docs

martong updated this revision to Diff 216579. martong marked an inline comment as done. martong added a comment. - Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66336/new/ https://reviews.llvm.org/D66336 Files: clang/docs

[PATCH] D66336: [ASTImporter] Add development internals docs

martong marked 6 inline comments as done. martong added a comment. Thanks for the review! Comment at: clang/docs/InternalsManual.rst:1470 +*templated* class (the ``CXXRecordDecl``) of a ``ClassTemplateDecl`` with +``ClassTemplateDecl::getTemplatedDec()``. And we can get back a

[PATCH] D66538: [AST] AST structural equivalence to work internally with pairs.

martong added a comment. > It looks like that the original code is correct in the decision of structural > equivalence of the original pair. If we have an (A,B) and (A,C) to compare, B > and C are in different decl chain, then (A,B) or (A,C) will be non-equivalent > (unless B and C are equivale

[PATCH] D66538: [AST] AST structural equivalence to work internally with pairs.

martong added a comment. The link for the diff went off, sorry about that. Here is the new link which is going to work: https://github.com/martong/clang/compare/NonEqDecls_cache_in_an_upper_level_0...martong:NonEqDecls_cache_in_an_upper_level?expand=1 Repository: rG LLVM Github Monorepo CHANG

[PATCH] D59692: [ASTImporter] Fix name conflict handling

martong marked an inline comment as done. martong added a comment. @shafik Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59692/new/ https://reviews.llvm.org/D59692 ___ cfe-commits mailing list cfe

[PATCH] D66538: [AST] AST structural equivalence to work internally with pairs.

martong added a comment. Ok. I like this patch because it eliminates the need for checking the redeclaration chains. Seems like it handles cycles and the simple `f(A,B)` vs `f(A,A)` cases properly too. (Not talking about caching now, probably we must remove the `NonEquivalentDecls` cache.) I'

[PATCH] D66538: [AST] AST structural equivalence to work internally with pairs.

martong added a comment. There is a third test which could be useful to test whether there is no faulty cache entries there: +TEST_F(StructuralEquivalenceCacheTest, DISABLED_NonEq) { + auto Decls = + makeTuDecls( + R"( +class A {}; +class B {

[PATCH] D62131: [ASTImporter] Remove NonEquivalentDecls from ASTImporter

martong marked an inline comment as done. martong added a comment. In D62131#1639168 , @balazske wrote: > Example about how to get wrong things into `NonEquivalentDecls`: > We want to compare `class C` for structural equivalence in the following > codes:

[PATCH] D62131: [ASTImporter] Remove NonEquivalentDecls from ASTImporter

martong abandoned this revision. martong added a comment. @a_sidorin Alexei, With Balazs we had a long discussion and I think we reached a consensus and common understanding. Now I try to summarize it. It turned out that this patch essentially renders the `NonEquivalentDecls` cache to be compl

[PATCH] D62131: [ASTImporter] Remove NonEquivalentDecls from ASTImporter

martong added a comment. > ... it is highly probable that if some pairs were ever non-equivalent they > will stay non-equivalent. This may not be true in case of LLDB, because usually they do a minimal import, and in a later phase they do a normal import which imports e.g. the members of a cla

[PATCH] D59692: [ASTImporter] Fix name conflict handling

martong updated this revision to Diff 217135. martong added a comment. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. - Use resulting Name from HandleNameConflict if set - Add ODRHandling strategies - Refactor tests, to avoid some code repetition Repository: rG LLVM Git

[PATCH] D59692: [ASTImporter] Fix name conflict handling with different strategies

martong added a comment. @shafik I have updated the patch with ODR handling strategies as per our discusson. For the record I copy our discussion here: On Sat, Aug 24, 2019 at 12:50 AM Shafik Yaghmour wrote: > Apologies, I missed this earlier! > > On Wed, Aug 21, 2019 at 2:44 AM Gábor Márto

[PATCH] D66538: [AST] AST structural equivalence to work internally with pairs.

martong added a comment. In D66538#1642999 , @balazske wrote: > In D66538#1642883 , @martong wrote: > > > There is a third test which could be useful to test whether there is no > > faulty cache entries there: > >

[PATCH] D66336: [ASTImporter] Add development internals docs

martong updated this revision to Diff 217171. martong added a comment. - Fix typo: getTemplatedDec -> getTemplatedDecl Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66336/new/ https://reviews.llvm.org/D66336 Files: clang/docs/InternalsManual.rst

[PATCH] D66336: [ASTImporter] Add development internals docs

martong marked 2 inline comments as done. martong added inline comments. Comment at: clang/docs/InternalsManual.rst:1470 +*templated* class (the ``CXXRecordDecl``) of a ``ClassTemplateDecl`` with +``ClassTemplateDecl::getTemplatedDec()``. And we can get back a pointer of the +"de

<    7   8   9   10   11   12   13   14   15   16   >