[PATCH] D122528: [clang][ASTImporter] Not using consumeError at failed import of in-class initializer.

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Thanks! Good catch! (For inexperienced readers, let me explain why I accepted the patch: If there is an error then the imported Decl is marked as erroneous, i.e an Error object is

[PATCH] D122525: [clang][ASTImporter] Fix an import error handling related bug.

martong added a comment. Nice catch! Comment at: clang/lib/AST/ASTImporter.cpp:8789-8791 +// The import path contains child nodes first. +// (Parent-child relationship is used here in sense of import +// dependency.) I think, this

[PATCH] D103094: [analyzer] Implemented RangeSet::Factory::castTo function to perform promotions, truncations and conversions

martong added a comment. In D103094#3403075 , @ASDenysPetrov wrote: > Ping. I am still interested in this! I am sorry for being inactive with it lately, I hope I'll have time soon to continue the review of this patch stack. CHANGES SINCE LAST ACTION

[PATCH] D121459: [analyzer] Remove HasAlphaDocumentation tablegen enum value

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Hahh, indeed. IMHO, it does not make sense to write an "alpha" documentation, either write it properly or do not write anything. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D122244: [analyzer] Turn missing tablegen doc entry of a checker into fatal error

martong accepted this revision. martong added a comment. Good. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122244/new/ https://reviews.llvm.org/D122244 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D122243: [analyzer][NFC] Introduce the checker package separator character

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

[PATCH] D121387: [analyzer] ClangSA should tablegen doc urls refering to the main doc page

martong added inline comments. Comment at: clang/utils/TableGen/ClangSACheckersEmitter.cpp:90-91 + + return (llvm::Twine("https://clang.llvm.org/docs/analyzer/checkers.html#;) + + CheckerFullName) .str(); > This patch will ensure that the doc

[PATCH] D121176: [ASTStructuralEquivalence] Add support for comparing ObjCCategoryDecl.

martong added a comment. Basically, this looks good to me. But my confidence with ObjC is low, so, I'd like @shafik as well to approve. Besides, seems like the pre-merge check fails with the new tests on Debian, could you please address that? Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D120824: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.

martong added a comment. In D120824#3369563 , @balazske wrote: > > I think that the problem may be related to the fact that the in-class > initializer is not used by the code in the "To" AST (in > //ctu-cxxdefaultinitexpr.cpp// the problem is with

[PATCH] D120824: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.

martong added a comment. Okay, thanks for the explanation. > The in-class initializer expression is not always stored in the AST, in the > ToTU it is missing initially. The field has the flag set that it contains > in-class initializer but the actual expression is not set yet (probably >

[PATCH] D120824: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. So, my understanding is that the issue stems from the fact that `hasInClassInitializer()` gave inconsistent results with `getInClassInitializer()` for previously imported nodes. Please

[PATCH] D117568: [Analyzer] Add docs to StdCLibraryFunctionArgsChecker

This revision was automatically updated to reflect the committed changes. Closed by commit rG280b43031ca0: [Analyzer] Add docs to StdCLibraryFunctionArgsChecker (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117568/new/

[PATCH] D116155: [clang][AST][ASTImporter] Set record to complete during import of its members.

martong added a subscriber: teemperor. martong added a comment. Adding Raphael @teemperor , he might have useful comments about the minimal mode. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116155/new/ https://reviews.llvm.org/D116155

[PATCH] D115521: [Templight] Don't display empty strings for names of unnamed template parameters

martong accepted this revision. martong added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115521/new/ https://reviews.llvm.org/D115521 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D117229: [Analyzer] Produce SymbolCast for pointer to integer cast

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:1022 +SVal clang::ento::SValBuilder::simplifySymbolCast(SymbolRef SE, + QualType CastTy) { ASDenysPetrov wrote: > And it'd

[PATCH] D117229: [Analyzer] Produce SymbolCast for pointer to integer cast

martong updated this revision to Diff 401274. martong marked an inline comment as done. martong added a comment. - Move the comment hunk Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117229/new/ https://reviews.llvm.org/D117229 Files:

[PATCH] D117568: [Analyzer] Add docs to StdCLibraryFunctionArgsChecker

martong added inline comments. Comment at: clang/docs/analyzer/checkers.rst:2361 +(void)ret; +clang_analyzer_eval(EOF <= x && x <= 255); // this reports TRUE + } NoQ wrote: > I recommend against using `clang_analyzer_eval` in user docs. Users aren't >

[PATCH] D117568: [Analyzer] Add docs to StdCLibraryFunctionArgsChecker

martong updated this revision to Diff 401230. martong marked 3 inline comments as done. martong added a comment. - Describe the different kind of constraints and limitations - Some rewording Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117568/new/

[PATCH] D117568: [Analyzer] Add docs to StdCLibraryFunctionArgsChecker

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

[PATCH] D115932: [Analyzer] Create and handle SymbolCast for pointer to integral conversion

martong added a comment. > I took Denys' patch D105340 and created a > prototype on top of that to create the SymbolCast in > SValBuilder::evalCastSubKind(loc::MemRegionVal V, Here is the new (alternative) patch: https://reviews.llvm.org/D117229 Repository:

[PATCH] D117229: [Analyzer] Produce SymbolCast for pointer to integer cast

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

[PATCH] D115932: [Analyzer] Create and handle SymbolCast for pointer to integral conversion

martong added a comment. Ah, I forgot to mention one last point: 5. Revert D115149 . We should never reach that problematic assertion once we produce the `SymbolCast`s. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D115932: [Analyzer] Create and handle SymbolCast for pointer to integral conversion

martong added a comment. Thanks for the review guys. Artem, I agree, that we should remove `LocAsInteger`. `LocaAsInteger` is a primitive handling of a specific cast operation (when we cast a pointer to an integer). The integration of `LocaAsInteger` into the `SymExpr` hierarchy is

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

martong added a comment. In D105340#3237430 , @ASDenysPetrov wrote: > In D105340#3232671 , @NoQ wrote: > >> This looks great with the option flag. Landing this patch will enable more >> people to test the new

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

martong added a comment. > I think for now it is enough to issue a warning of using these functions, and > not suggest a replacement. Should we add an option to the checker to also > check for these functions? IMHO, it is okay to start with just simply issuing the warning. Later we might

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

martong added a comment. In D114622#3200678 , @NoQ wrote: > These checks are almost 2000 lines of code each and it looks like all they do > is figure out if two statements are the same and we have a very flexible > reusable solution for this sort of

[PATCH] D114706: [analyzer] Fix sensitive argument logic in GenericTaintChecker

martong added a comment. > The semantics of taint sinks is that if ANY of the arguments is tainted, a > warning should be emmitted. Before this change, if there were multiple > arguments that are sensitive, and if the first arg is not tainted, but any of > the noninitial are tainted, a warning is

[PATCH] D115355: Fix build failure with GCC 11 in C++20 mode

This revision was automatically updated to reflect the committed changes. Closed by commit rG2ccf0b76bcaf: Fix build failure with GCC 11 in C++20 mode (authored by Godin, committed by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D115521: [Templight] Don't display empty strings for names of unnamed template parameters

martong added inline comments. Comment at: clang/lib/Frontend/FrontendActions.cpp:501 + +if (const auto *Decl = dyn_cast(NamedTemplate)) { + if (const auto *R = dyn_cast(Decl)) { martong wrote: > Should this handle `EnumDecl`s as well? An enum

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

martong added a comment. In D55134#3224339 , @ychen wrote: > FWIW, ASTUnit seems to save an effective triple while the current AST uses > the nominal triple. (for example, `armv7a-xx-xx-eabihf` vs > `armv7a-xx-xx-eabi`). I'm not sure if there is a good

[PATCH] D116155: [clang][AST][ASTImporter] Set record to complete during import of its members.

martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:2021 + // fields imported) at that time without multiple AST import passes. + To->setCompleteDefinition(true); // Complete the definition even if error is returned. balazske wrote: >

[PATCH] D115932: [Analyzer] Create and handle SymbolCast for pointer to integral conversion

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:417 + // the member function of SValBuilder (?) + if (symRHS) +if (auto RLocAsInt = RHS.getAs()) { We should handle LHS as well. Repository: rG LLVM Github

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

martong added a comment. In D115149#3181580 , @NoQ wrote: > Like, that's the whole reason why `nonloc::LocAsInteger` exists: so that we > could cast a pointer to an integer and actually have a way to represent the > resulting value as `NonLoc`. > I'm

[PATCH] D115932: [Analyzer] Create and handle SymbolCast for pointer to integral conversion

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

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

martong added a comment. In D115149#3195377 , @ASDenysPetrov wrote: > @martong > Thanks for clarification. > >> TLDR, it is recommended to use the arcanist. > > I'm not able to use arcanist. It doesn't work on Windows (at least I've tryed > several

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

martong added a comment. In D115149#3192725 , @ASDenysPetrov wrote: > @martong wrote: > >> Denis, you can see in the `Revision Contents` that Diff 3 has the baseline >> commit `63a6348`. When I check out `63a6348` then the newly added test file >>

[PATCH] D46081: [analyzer] Expand conversion check to check more expressions for overflow and underflow

This revision was automatically updated to reflect the committed changes. Closed by commit rGbd9e23943a22: [analyzer] Expand conversion check to check more expressions for overflow and… (authored by martong). Changed prior to commit: https://reviews.llvm.org/D46081?vs=144202=394502#toc

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

martong added a comment. In D115149#3188743 , @ASDenysPetrov wrote: >> @steakhal >> I don't get this one. I've provided a bunch of tests, even annotated with >> `no-crash` comments where we crashed prior to this change. > > I wasn't able to catch any

[PATCH] D114938: [Analyzer][NFCi] SValBuilder: Simlify a SymExpr to the absolute simplest form

This revision was automatically updated to reflect the committed changes. Closed by commit rG978431e80b61: [Analyzer] SValBuilder: Simlify a SymExpr to the absolute simplest form (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D114938: [Analyzer][NFCi] SValBuilder: Simlify a SymExpr to the absolute simplest form

martong added a comment. In D114938#3174331 , @steakhal wrote: > You mentioned in the summary that there are different places where > simplification-like machinary kicks in, which hindered the test case > synthesis. What places did you refer to

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

martong added a comment. In D115149#3175068 , @NoQ wrote: >> It can happen if the `Loc` was perfectly constrained to a concrete >> value (`nonloc::ConcreteInt`) > > This shouldn't happen. It should be `loc::ConcreteInt` which is, well, a > `Loc`. I see

[PATCH] D114418: [clang][ASTImporter] Update lookup table correctly at deduction guides.

This revision was automatically updated to reflect the committed changes. Closed by commit rG341a30a4ba4b: [clang][ASTImporter] Update lookup table correctly at deduction guides. (authored by balazske, committed by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D114418: [clang][ASTImporter] Update lookup table correctly at deduction guides.

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

[PATCH] D114938: [Analyzer] SValBuilder: Simlify a SymExpr to the absolute simplest form

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:51 + // sub-trees and if a value is a constant we do the constant folding. + SVal simplifySValImpl(ProgramStateRef State, SVal V); + steakhal wrote: > What about

[PATCH] D113753: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:372 QualType resultTy) { NonLoc InputLHS = lhs; NonLoc InputRHS = rhs; steakhal wrote: > @martong, you simplified the

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

martong accepted this revision. martong added a comment. Nice, assiduous work! The tests are awesome! LGTM, with minor revisions. Please check out my suggestions about the tests' formatting and there are those disturbing (LHS, RHS) swaps in the comments. I am going to continue with the next

[PATCH] D114938: [Analyzer] SValBuilder: Simlify a SymExpr to the absolute simplest form

martong updated this revision to Diff 391352. martong marked 2 inline comments as done. martong added a comment. - Rename simplifySValImpl to simplifySValOnce - Remove a comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114938/new/

[PATCH] D113753: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN

martong added a comment. Here is a smaller reproducer: void bar(short k) { ++k; // k1 = k0 + 1 assert(k == 1); // k1 == 1 --> k0 == 0 (long)k << 16; // k0 + 1 << 16 } And the explanation is the following. With this patch, when the analyzer evaluates the `(long)k

[PATCH] D114938: [Analyzer] SValBuilder: Simlify a SymExpr to the absolute simplest form

martong added a comment. Measurement results are attached, I think there is no relevant difference. F20847935: another_fixpoint_before_merge_2.png F20847949: stats.html Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D114938: [Analyzer] SValBuilder: Simlify a SymExpr to the absolute simplest form

martong created this revision. martong added a reviewer: 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. martong requested review

[PATCH] D114887: [Analyzer][solver] Simplification: Do a fixpoint iteration before the eq class merge

martong added a comment. In D114887#3165397 , @steakhal wrote: > Also, please provide the coverage of the new test case (only excercising the > new test case not the whole test file) I really want to make sure that it > will cover the loop as it

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

martong added a comment. I've had the confidence to commit D114887 , the test should not be flaky anymore. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114619/new/ https://reviews.llvm.org/D114619

[PATCH] D114887: [Analyzer][solver] Simplification: Do a fixpoint iteration before the eq class merge

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG20f8733d4b8d: [Analyzer][solver] Simplification: Do a fixpoint

[PATCH] D114887: [Analyzer][solver] Simplification: Do a fixpoint iteration before the eq class merge

martong added a comment. I am attaching the performance measurement results: F20834663: removemember_revert_with_another_fixpoint.png F20834673: stats.html There is no any noticeable difference. I am going to commit

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

martong added a comment. @thakis, if the issue is really disturbing and cannot wait until the review of D114887 finishes then please do a revert of this patch. But then the dependent child patch D103317 needs to be reverted

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

martong added a comment. In D114619#3164898 , @thakis wrote: > The test sometimes fails flakily: http://45.33.8.238/macm1/22813/step_7.txt Thanks @thakis for the report, I am aware of the issue and addressing that in https://reviews.llvm.org/D114887

[PATCH] D114887: [Analyzer][solver] Simplification: Do a fixpoint iteration before the eq class merge

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2218 +LLVM_NODISCARD +static SVal simplifyUntilFixpoint(SValBuilder , ProgramStateRef State, + const SymbolRef Sym) { Perhaps

[PATCH] D114887: [Analyzer][solver] Simplification: Do a fixpoint iteration before the eq class merge

martong created this revision. martong added a reviewer: 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. martong requested review

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

This revision was automatically updated to reflect the committed changes. Closed by commit rG0a17896fe6fd: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in… (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

This revision was automatically updated to reflect the committed changes. Closed by commit rGf02c5f347831: [Analyzer][solver] Do not remove the simplified symbol from the eq class (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

martong added a comment. In D114619#3155363 , @steakhal wrote: > I see. Simplification is always good. Let's measure and compare the runtime > characteristics before moving forward. Here they are. Seems like the runtime and memory consumption are

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2226 + // + // Empirical measurements show that if we relax assumption G then the + // runtime does not grow noticeably. This is most probably because the

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

martong updated this revision to Diff 390349. martong marked 4 inline comments as done. martong added a comment. - Update a comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114619/new/ https://reviews.llvm.org/D114619 Files:

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong marked an inline comment as done. martong added inline comments. Comment at: clang/test/Analysis/svalbuilder-simplify-compound-svals.cpp:75 + if (b * b == 1) +; // no-crash (assertion should not be violated) + } steakhal wrote: > I guess,

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong updated this revision to Diff 390343. martong marked an inline comment as done. martong added a comment. - Update a test case - Rebase to the parent patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103317/new/

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong added a comment. > Please repeat the measurement for `openssl`. There must have been some > interference in the memory consumption. Aside from that the results look > great. I did. Average memory usage is unaffected. F20722167: svalbuilder_improvements_openssl.png

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

martong created this revision. martong added a reviewer: 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. martong requested review

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong added a comment. I've added a new test case that triggers an assertion. That problem is getting fixed in a new parent patch, which I am about to add to the stack very soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103317/new/

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong updated this revision to Diff 389936. martong added a comment. - Remove superfluous param (int a) from the new test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103317/new/ https://reviews.llvm.org/D103317 Files:

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong updated this revision to Diff 389935. martong marked an inline comment as done. martong added a comment. - Add new test case (for no-crash) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103317/new/ https://reviews.llvm.org/D103317 Files:

[PATCH] D114418: [clang][ASTImporter] Update lookup table correctly at deduction guides.

martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:6082-6085 + // "from" context. Because these DeclContext values look already not stable + // and unimportant this change looks acceptable. + // For these reasons the old DeclContext must be saved to

[PATCH] D114256: [clang-tidy] Fix crashing altera-struct-pack-align on invalid RecordDecls

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114256/new/ https://reviews.llvm.org/D114256

[PATCH] D114418: [clang][ASTImporter] Update lookup table correctly at deduction guides.

martong added a comment. > Probably this is a bug in the AST creation but currently it works this way. I don't think this is a bug, deduction guides have a convoluted implementation. See the related DeclContext issue with them when local typdefs are involved: https://reviews.llvm.org/D92209

[PATCH] D114454: [NFC][AIX]Disable unstable CSA tests failing on AIX

martong edited reviewers, added: steakhal; removed: vsavchenko. martong added a comment. vsavchenko is inactive, presumably he is no longer working with CSA Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114454/new/ https://reviews.llvm.org/D114454

[PATCH] D114256: [clang-tidy] Fix crashing altera-struct-pack-align on invalid RecordDecls

martong added a comment. What happens if this checker runs on a forward declared class? struct Foo; I'd expect the pack/alignment info is missing also in that case. Comment at: clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.cpp:54 + // Ignore invalid decls to

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong marked an inline comment as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1105 - // FIXME: Add support for SymExprs. return nullptr; steakhal wrote: > Where did you address this FIXME? I

[PATCH] D113118: [clang][AST] Check context of record in structural equivalence.

martong accepted this revision. martong added a comment. Still LGTM! Let's land it! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113118/new/ https://reviews.llvm.org/D113118 ___ cfe-commits mailing

[PATCH] D114418: [clang][ASTImporter] Update lookup table correctly at deduction guides.

martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:6081 + // FunctionTemplateDecl objects are created, but in different order. In this + // way DeclContext of these template parameters may change relative to the + // "from" context. Because these

[PATCH] D113753: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN

martong added a comment. I'm attaching the coverage of the new test file for the related change: 375 : // Constraints may have changed since the creation of a bound SVal. Check if 376 : // the values can be simplified based on those new constraints. 377

[PATCH] D113753: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG12887a202404: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN (authored by martong). Repository: rG LLVM Github Monorepo

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong added inline comments. Comment at: clang/test/Analysis/svalbuilder-simplify-compound-svals.cpp:32 + clang_analyzer_eval(x + y * z == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(y * z == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(x == 0);

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong added a comment. In D103317#3127318 , @steakhal wrote: > To me at least, the patch looks good. > Please post some comparative measurements to demonstrate it won't introduce > runtime regression. Sure! F20586670: stats.html

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1144 + : (SVal)SVB.makeIntVal(*Const); + return SVal(); +} steakhal wrote: > Let's be explicit about it. Good

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong updated this revision to Diff 389198. martong marked 6 inline comments as done. martong added a comment. - Return explicitly with UndefinedVal - Unify test cases (return 0 -> return) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103317/new/

[PATCH] D113753: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN

martong updated this revision to Diff 389186. martong marked an inline comment as done. martong added a comment. - Add new test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113753/new/ https://reviews.llvm.org/D113753 Files:

[PATCH] D113753: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN

martong marked 2 inline comments as done. martong added a comment. I'm attaching the coverage of the new test file for the related change: 375 : // Constraints may have changed since the creation of a bound SVal. Check if 376 : // the values can be simplified

[PATCH] D113754: [Analyzer][Core] Simplify IntSym in SValBuilder

This revision was automatically updated to reflect the committed changes. Closed by commit rGffc32efd1cd6: [Analyzer][Core] Simplify IntSym in SValBuilder (authored by martong). Changed prior to commit: https://reviews.llvm.org/D113754?vs=386800=388927#toc Repository: rG LLVM Github

[PATCH] D113754: [Analyzer][Core] Simplify IntSym in SValBuilder

martong added a comment. In D113754#3127245 , @steakhal wrote: > Great stuff. Have you checked the coverage? Thanks for the review! So, here are the coverage results for the new test file: 1186 2 : SVal VisitIntSymExpr(const IntSymExpr

[PATCH] D46081: [analyzer] Expand conversion check to check more expressions for overflow and underflow

martong added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:102 } - } else if (isa(Parent)) { + } else if (isa(Parent) || isa(Parent)) { +if (!Cast->IgnoreParenImpCasts()->isEvaluatable(C.getASTContext())) { CHANGES

[PATCH] D46081: [analyzer] Expand conversion check to check more expressions for overflow and underflow

martong added a comment. @whisperity Thank you for pinging! This seems still feasible. Besides, this is valuable for us, as it could pass some SEI CERT test cases, notably from INT31

[PATCH] D113118: [clang][AST] Check context of record in structural equivalence.

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Thanks @balazske for updating the tests, I think this addresses @shafik 's questions. LGTM! Ping @shafik Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D114006: [analyzer][NFC] Enable access to CodeGenOptions from analyzer's instances

martong accepted this revision. martong added a comment. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114006/new/ https://reviews.llvm.org/D114006 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

martong added a comment. Thanks for the update, I am okay with the `.cpp` file, now I continue the review with the tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99797/new/ https://reviews.llvm.org/D99797 ___ cfe-commits mailing list

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

martong added a comment. Gentle ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99797/new/ https://reviews.llvm.org/D99797 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D114025: [clang][NFC] Inclusive terms: replace some uses of sanity in clang

martong added a comment. Do we have a comprehensive list of non-inclusive terms and their inclusive correspondent somewhere available? I mean `master` -> `main`, `white list` -> `inclusive list`, `sanity` -> `validation`, ... I'd assume that we go through that list, and that could give me a

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

martong added a comment. In D104550#3139239 , @stevewan wrote: > In D104550#2849582 , @vsavchenko > wrote: > >> In D104550#2849561 , >> @DavidSpickett wrote: >> >>>

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong updated this revision to Diff 386806. martong edited the summary of this revision. martong added a comment. - Update in comments: `Unknown` -> `Undef` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103317/new/

[PATCH] D113754: [Analyzer][Core] Simplify IntSym in SValBuilder

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

[PATCH] D113753: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN

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

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

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1155-1156 -// TODO: Support SymbolCast. Support IntSymExpr when/if we actually -// start producing them. ASDenysPetrov wrote: > ASDenysPetrov wrote: > >

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