[PATCH] D92101: [Clang][Sema] Attempt to fix CTAD faulty copy of non-local typedefs

martong updated this revision to Diff 308922. martong added a comment. - Add to MaterializedTypedefs only when copying the Decl Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92101/new/ https://reviews.llvm.org/D92101 Files: clang/lib/Sema/SemaTe

[PATCH] D92101: [Clang][Sema] Attempt to fix CTAD faulty copy of non-local typedefs

martong added a comment. @rsmith I have moved the check into `TransformTypedefType`. We do not add the typedef to the `MaterializedTypedefs` list if it's context is dependent because later we set the deduction guide as the DC for all typedefs in the list: for (auto *TD : MaterializedTypedefs)

[PATCH] D92101: [Clang][Sema] Attempt to fix CTAD faulty copy of non-local typedefs

martong added a comment. In D92101#2423685 , @shafik wrote: > Should we add a test for the `regular function template` case as well? I think this issue is specific for a `CXXDeductionGuideDecl` which is a `FunctionDecl`. I am not sure if I can follow wha

[PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

martong added a subscriber: balazske. martong added a comment. Hey Raphael, thanks for looking into the CTU crash! I also had a look and I could reproduce that on Linux Ubuntu 18.04 with GCC 7. I think the discrepancy stems from GCC's libstdc++ and MacOS's libc++ implementation differences of `

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

martong created this revision. martong added a reviewer: steakhal. Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, whisperity. Herald added a reviewer: Szelethus.

[PATCH] D91997: APINotes: add bitcode format schema definitions

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

[PATCH] D92432: [analyzer] Add a thin abstraction layer between libCrossTU and libAnalysis.

martong accepted this revision. martong added a comment. Looks good, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92432/new/ https://reviews.llvm.org/D92432 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.o

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

martong marked 3 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1771 +.Case({ReturnValueCondition(LessThanOrEq, ArgNo(2)), + ReturnValueCondition(WithinRange, Rang

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

martong marked 2 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:2076 Summary(NoEvalCall) +.Case({ReturnValueCondition(WithinRange, Range(-1, 0))}) .ArgConstrai

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

martong updated this revision to Diff 308951. martong marked an inline comment as done. martong added a comment. - Remove comments - Hoist Range(-1,0) to ReturnsZeroOrMinusOne Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92474/new/ https://reviews

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGb40b3196b321: [analyzer][StdLibraryFunctionsChecker] Add return value constraint to functions… (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D92101: [Clang][Sema] Attempt to fix CTAD faulty copy of non-local typedefs

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 rG1e14588d0f68: [Clang][Sema] Attempt to fix CTAD faulty copy of non-local typedefs (authored by martong). C

[PATCH] D92209: [ASTImporter] Support CXXDeductionGuideDecl with local typedef

martong marked an inline comment as done. martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:2522 + // Add to the lookupTable because we could not do that in MapImported. + Importer.AddToLookupTable(ToTypedef); + shafik wrote: > I am not s

[PATCH] D92209: [ASTImporter] Support CXXDeductionGuideDecl with local typedef

martong marked an inline comment as done. martong added a comment. > I was thinking about to create a separate ASTImporter implementation > specifically for CTU and LLDB could have it's own implementation. For that we > just need to create an interface class with virtual functions. One implemen

[PATCH] D92600: [ASTImporter] Add support for importing GenericSelectionExpr AST nodes.

martong added a comment. It might be worth to extend StmtComparer as well with this Expr. I mean, probably two selections exprs should be considered equal if thei

[PATCH] D92600: [ASTImporter] Add support for importing GenericSelectionExpr AST nodes.

martong added a comment. In D92600#2433263 , @martong wrote: > It might be worth to extend StmtComparer > > as well with

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

martong created this revision. martong added a reviewer: steakhal. Herald added subscribers: ASDenysPetrov, Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, whisperity. Herald added a reviewer: Szelethus. martong requ

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

martong updated this revision to Diff 309931. martong marked an inline comment as done. martong added a comment. - Use -1 for mmap too Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92764/new/ https://reviews.llvm.org/D92764 Files: clang/lib/Stat

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

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1737 .ArgConstraint( ArgumentCondition(4, WithinRange, Range(0, IntMax; steakhal wrote: > `mmap` should have the same

[PATCH] D92771: [analyzer][StdLibraryFunctionsChecker] Add more return value contraints

martong created this revision. martong added a reviewer: steakhal. Herald added subscribers: ASDenysPetrov, Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, whisperity. Herald added a reviewer: Szelethus. martong requ

[PATCH] D92209: [ASTImporter] Support CXXDeductionGuideDecl with local typedef

martong added a comment. Ping. @teemperor please take a look if you have some time. This is a really important patch which may influence some future directions regarding the ASTImporter. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92209/new/ htt

[PATCH] D88859: APINotes: add APINotesYAMLCompiler

martong added a comment. > as it is something which is actually in production already > ... > At the very least we need it for compatibility - this is already a shipping > feature. I value that you guys have a prototype on a fork that is being used in production. But, upstreaming and these revi

[PATCH] D88859: APINotes: add APINotesYAMLCompiler

martong added a reviewer: NoQ. martong added a subscriber: NoQ. martong added a comment. Herald added a subscriber: Charusso. Adding @NoQ as a reviewer, he might have some valuable comments, as he is very keen on this feature. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D89318: [ASTImporter] Fix crash caused by unset AttributeSpellingListIndex

martong added a comment. Unfortunately, I could not reproduce the mentioned crash on our macOS machine. The mentioned test just passed with the output below. I gave up. myuser@msmarple ~/llvm3/build/release_assert $ /usr/local/Frameworks/Python.framework/Versions/3.8/bin/python3.8 /Users/myu

[PATCH] D89528: [clang][test] Fix prefix operator++ signature in iterators

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. Ok, thanks for the context. LG! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89528/new/ https://reviews.

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

martong added a comment. > 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. This is not going to be easy. I guess we should cover both implicit and explicit

[PATCH] D88859: APINotes: add APINotesYAMLCompiler

martong added a comment. In D88859#2354377 , @gribozavr2 wrote: >> I am having a hard time to accept "this is how it is implemented in our >> fork" as a technical argument. Besides, I am not sure how could the Clang >> community benefit about being backw

[PATCH] D63279: [Analyzer] Unroll for-loops where the upper boundary is a variable with know value

martong added a comment. FYI, I am working on repeating and reevaluating Peter's measurements with the current master. And if possible then I'd like to extend the measures for new C and C++ projects. I am planning to sh

[PATCH] D88859: APINotes: add APINotesYAMLCompiler

martong added a comment. In D88859#2359399 , @compnerd wrote: >> I'd like to have maximum flexibility from the submitter by willing to change >> the details of the implementation > > I don't think that is a fair expectation - there are plenty of times whe

[PATCH] D88859: APINotes: add APINotesYAMLCompiler

martong added inline comments. Comment at: clang/lib/APINotes/APINotesYAMLCompiler.cpp:33 +namespace yaml { +template <> +struct ScalarEnumerationTraits { Could you please clang-format the code? The `Lint: Pre-merge checks` are all over the code and makes the re

[PATCH] D88859: APINotes: add APINotesYAMLCompiler

martong added inline comments. Comment at: clang/include/clang/APINotes/Types.h:25 +/// auditing. +enum class EnumExtensibilityKind { + None, compnerd wrote: > martong wrote: > > compnerd wrote: > > > martong wrote: > > > > compnerd wrote: > > > > > martong wrot

[PATCH] D88859: APINotes: add APINotesYAMLCompiler

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. Looks good to me now! Thanks for addressing my concerns. Comment at: clang/tools/apinotes-test/APINotesTest.cpp:15 + +static llvm::cl::

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

martong added a comment. Post commit looks good to me as well. @steakhal Nice work! @shafik Thanks for the review! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102640/new/ https://reviews.llvm.org/D102640 _

[PATCH] D102241: [clang] p1099 4/5: using enum EnumTag

martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:4654 + + return ImportUsingShadowDecls(D, ToUsingEnum); } Could it work if you imported the shadow declarations **before** creating the `UsingEnumDecl`? I yes, then that would be the pref

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

martong added a comment. Thank you @OikawaKirie for working on this many CTU related patches! I am going to find time for a thorough review and going to pursue @gamesh411 as well to do the same! On the other hand, it would be really useful if you could build a "Stack" from these patches, I mean

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

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

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

martong added a comment. In D102696#2766337 , @NoQ wrote: > Ok so the state has enough constraints. It's enough to know that `$x + $y == > 0` and `$y == 0` in order to deduce that `$x + 0 == 0`. But you're saying > that we don't know how to infer it s

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

martong added a comment. > In this case the equations are $y == 0 and $x + 0 == 0 which is much easier > to solve. Yes, you are right. > This happens for the same reason that your patch is needed in the first > place: we're eagerly substituting a constant. Absolutely, that's the point. On the

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

martong added a comment. First of all, thank you for the patch! We had a meeting with my colleges (@steakhal, @gamesh411) and we agreed in the following. This issue you are trying to solve here is indeed a serious problem, but we'd like to suggest an alternative and perhaps more durable solutio

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

martong added a comment. Thank you for the patch! Though, the idea is nice, there is a serious technical obstacle here: we cannot use the clangTooling lib as a dependency of the CTU lib because that would introduce a circular dependency. Actually, this was the reason for introducing the invoca

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

martong added a comment. First of all, thank you for the patch! We had a meeting with my colleges (@steakhal, @gamesh411) and we agreed in the following. This is indeed an issue and the fix is okay. About the test, we'd like to ask if you could create a small test lib with its own 'open' functio

[PATCH] D102062: [analyzer][ctu] Append ctu-dir to ctu-invocation-list for non-absolute paths

martong added a comment. First of all, thank you for the patch! We had a meeting with my colleges (@steakhal, @gamesh411) and we agreed in the following. We'd like to keep the current behavior because this way the behavior is similar that we got used to with any other command line tools. Repo

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

martong updated this revision to Diff 346966. martong added a comment. - Add test case for commutativity - Add test case for Valeriy's case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102696/new/ https://reviews.llvm.org/D102696 Files: clang/l

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

martong added a comment. In D102696#2768857 , @vsavchenko wrote: > My take on this: for whatever approach, we definitely will be able to > construct a more nested/complex example so that it doesn't work. > For this patch, I'm wondering about something l

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

martong added a comment. In D102696#2769465 , @NoQ wrote: > In D102696#2767894 , @martong wrote: > >>> This happens for the same reason that your patch is needed in the first >>> place: we're eagerly substituting

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

martong added a comment. > As for the solver, it is something that tormented me for a long time. Is > there a way to avoid a full-blown brute force check of all existing > constraints and get some knowledge about symbolic expressions by constraints > these symbolic expressions are actually part

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

martong added a comment. In D102696#2773340 , @vsavchenko wrote: > In D102696#2773304 , @martong wrote: > >>> As for the solver, it is something that tormented me for a long time. Is >>> there a way to avoid a f

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

martong added a comment. Alright, I see your point. I agree that solving the problem of "$a +$b +$c constrained and then $a + $c got constrained" requires canonicalization. However, canonicalization seems to be not trivial to implement. And there are some other easier cases that I think we could (

[PATCH] D103231: [clang][AST] Set correct DeclContext in ASTImporter lookup table for ParmVarDecl.

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. Thanks, looks good to me, with a nit in the tests. Comment at: clang/unittests/AST/ASTImporterTest.cpp:6199 + EXPECT_TRUE(ImportedF);

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

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

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

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

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

martong abandoned this revision. martong added a comment. Abandoning in favor of https://reviews.llvm.org/D103314 https://reviews.llvm.org/D103317 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102696/new/ https://reviews.llvm.org/D102696 _

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

martong added a comment. Thanks Valeriy for the quick review and guidance! I am planning to do the changes and continue next week :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103314/new/ https://reviews.llvm.org/D103314 __

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

martong marked an inline comment as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1561 + return nullptr; + +ConstraintMap CM = getConstraintMap(State); vsavchenko wrote: > Also I think we can in

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

martong updated this revision to Diff 348808. martong marked an inline comment as done. martong added a comment. - Merge the simplified symbol to the old class Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103314/new/ https://reviews.llvm.org/D1033

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

martong marked 5 inline comments as done. martong added a comment. In D103314#2789754 , @vsavchenko wrote: > I had another thought, `merge` is usually called in situations when we found > out that two symbols should be marked equal (and checked that it'

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

martong updated this revision to Diff 348945. martong marked 2 inline comments as done. martong added a comment. - Simplify equivalence classes when iterate over ClassMap, simplify constraints by iterating over the ConstraintsMap Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

martong marked 2 inline comments as done. martong added a comment. I was wondering if there is a direct way to check the equivalence classes? I am thinking about to add a `clang_annalyzer_dump_equivalence_classes` function to the ExprInspection checker. Repository: rG LLVM Github Monorepo CH

[PATCH] D102914: [analyzer] Make checker silencing work for non-pathsensitive bug reports

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. This makes sense, and looks good to me! Though, my review confidence is weak. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102914/new/ https://reviews.llvm.org/D102914 ___

[PATCH] D102492: [clang][AST] Add support for BindingDecl to ASTImporter.

martong added a comment. I like it, though I've found a nit. Comment at: clang/lib/AST/ASTImporter.cpp:2301 + ToD->setLexicalDeclContext(LexicalDC); + DC->addDeclInternal(ToD); + ToD->setBinding(ToType, ToBinding); Should we use rather `addDeclToContexts` ?

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

martong marked 3 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1559 + // absolute minimum. + LLVM_NODISCARD ProgramStateRef simplifyEquivalenceClass( + ProgramStateRef State, EquivalenceClass Cl

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

martong updated this revision to Diff 349261. martong marked 3 inline comments as done. martong added a comment. - Add isEqualTo and simplify members to EquivalenceClass Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103314/new/ https://reviews.llvm

[PATCH] D75041: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with mixability because of implicit conversions

martong added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:657-667 + // Certain kinds unfortunately need to be side-stepped for canonical type + // matching. + if (LType->getAs() || RType->getAs()) { +// Unfortunate

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

martong updated this revision to Diff 349352. martong added a comment. I am terribly sorry, but I uploaded an unfinished Diff previously, please disregard that. So these are the changes: - Add isEqualTo and simplify members to EquivalenceClass Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D103231: [clang][AST] Set correct DeclContext in ASTImporter lookup table for ParmVarDecl.

martong added a comment. In D103231#2795794 , @balazske wrote: > Added `contains` for correct check of `ASTImporterLookupTable` content. Okay, that looks good, but I just realized we should not have "bare" assertions. Could you please add some explanato

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

martong added a comment. Great initiative! You haven't mention `markInteresting` functions, but I think it would be really important to incorporate that functionality here as well. IMHO, `markInteresting` shouldn't have been part of the public API ever, Checkers should have been calling only th

[PATCH] D75041: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with mixability because of implicit conversions

martong added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:122 #define FB(Name, K) MIX_##Name = (1ull << (K##ull - 1ull)) FB stands for FunnyBitmask? Could you please either describe that in a comment

[PATCH] D95244: [clang][AST] Handle overload callee type in CallExpr::getCallReturnType.

martong added a comment. Hi Balázs, Could you please create a lit test file which ignites the related crash? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95244/new/ https://reviews.llvm.org/D95244 ___

[PATCH] D95244: [clang][AST] Handle overload callee type in CallExpr::getCallReturnType.

martong added reviewers: rsmith, majnemer. martong added subscribers: majnemer, rsmith. martong added a comment. Adding reviewers. @rsmith as code owner, @majnemer based on git blame of CallExpr::getCallReturnType. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

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

martong accepted this revision. martong added a comment. Still looks good to me! Thanks for handling the pragma cases! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93222/new/ https://reviews.llvm.org/D93222 ___ cfe-commits mailing list cfe-c

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

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Still looks good. (The parent is an NFC and only this patch changes the behavior (and only if the cmdline flag is there), right? So, I'd expect the big impact from this patch.) CHANGES SIN

[PATCH] D109836: [Analyzer] ConversionChecker: track back the cast expression

This revision was automatically updated to reflect the committed changes. Closed by commit rG96ec9b6ff2f0: [Analyzer] ConversionChecker: track back the cast expression (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109836/new/

[PATCH] D109836: [Analyzer] ConversionChecker: track back the cast expression

martong added a comment. In D109836#3002811 , @steakhal wrote: > LGTM. > Thanks for fixing this. Thanks for the review! > Should this checker remain in alpha in its current form? > WDYT? @all There is no clear explanation for this checker being alpha.

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

martong added a reviewer: steakhal. martong added a subscriber: steakhal. martong added a comment. @steakhal, could you please take a look? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109608/new/ https://reviews.llvm.org/D109608

[PATCH] D91948: [WIP][analyzer][doc] Add Container- and IteratorModeling developer docs

martong added a comment. Herald added a subscriber: manas. Nice work! Comment at: clang/docs/analyzer/developer-docs/ContainerModeling.rst:125 +The begin and end symbols are conjured and are completely unrelated to the region of +the container. For each region we store the onl

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

martong accepted this revision. martong added a comment. Herald added a subscriber: rnkovacs. Look good! Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109608/new/ https://reviews.llvm.org/D109608 ___

[PATCH] D104285: [analyzer] Retrieve a value from list initialization of constant array declaration in a global scope.

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1696 + const llvm::APSInt &Idx = CI->getValue(); + const uint64_t I = static_cast(Idx.getExtValue()); + // Use `getZExtValue` because array extent can n

[PATCH] D104285: [analyzer] Retrieve a value from list initialization of constant array declaration in a global scope.

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1696-1697 + const auto I = static_cast(Idx.getExtValue()); + // Use `getZExtValue`

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

martong added a comment. This is promising! Gentle ping @manas @vsavchenko 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-comm

[PATCH] D97874: [analyzer] Improve SVal cast from integer to bool using known RangeSet

martong added a comment. Herald added a subscriber: manas. @ASDenysPetrov I think the dependent patch https://reviews.llvm.org/D97296 is too much and contains unnecessary things for this change. If you could you please incorporate the minimum needed changes from that patch to here then this pat

[PATCH] D97960: [clang-tidy] bugprone-signal-handler improvements: display call chain

martong added a comment. So, about the tests, gentle ping @njames93 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97960/new/ https://reviews.llvm.org/D97960 ___ cfe-commits mailing list cfe-commits@lists

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

martong added a comment. Herald added a subscriber: manas. In D86295#2539431 , @steakhal wrote: > In D86295#2519851 , @ASDenysPetrov > wrote: > >> What about this change? Did you make more measurements? > > IMO it

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

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

[PATCH] D104285: [analyzer] Retrieve a value from list initialization of constant array declaration in a global scope.

martong added a comment. In D104285#3018257 , @ASDenysPetrov wrote: > @martong > BTW, this patch is the first one in the stack. There are also D107339 > and D108032 > . You could also

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

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

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

martong added a comment. I had to move the definition of `ConstraintAssignor` after the definition of `RangeConstraintManager` b/c I am using `assumeSymNE` in the new logic. Unfortunately, the diff does not show clearly the changes inside the moved hunk, so I try to indicate the important chang

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

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

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

martong updated this revision to Diff 374741. martong added a comment. - Break out the movement of RangeConstraintManager into a parent patch, this way the diff here is clearly visible and makes the review easier. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

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

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. I agree with @steakhal , that perhaps we could come up with a somewhat more compact and elegant implementation in the future. Nevertheless, I think this patch in its current form is already

[PATCH] D110398: [clang][ASTImporter] Import ConstructorUsingShadowDecl correctly.

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Nice work! Thanks for the assiduous tests! Comment at: clang/unittests/AST/ASTImporterTest.cpp:6827-6828 +FirstDeclMatcher().match( +TU, usingShadowDecl

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

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

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

martong added a comment. In D106102#3019921 , @manas wrote: > I haven't tried specializing that `VisitBinaryOperator` method which converts > Ranges from RangeSets (as @vsavchenko mentioned). Should this case for NE > stay here in the switch or move? P

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

martong added a reviewer: steakhal. martong added a comment. Adding @steakhal as reviewer because of other reviewers inactivity. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106102/new/ https://reviews.llvm.org/D106102 ___

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

martong added a comment. In D110528#3026921 , @balazske wrote: > If the `ASTImporter` and parts of `ASTNodeImporter` are made public API then > it can be possible to add generation of import code to the **.td** files, > this should be the best solution.

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

martong added a comment. Thanks Vince! Nice work! Do you think it would be worth to have a test that checks the equality of a double pointer and a bi-dimensional array? Something like: void struct_pointer_canon(struct s **ps) { struct s ss = **ps; clang_analyzer_eval(&(ps[0][0].v) ==

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

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

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

martong added a comment. In D106102#3028584 , @manas wrote: > The pre-merge checks fail due to the patch being unable to get applied. The > troubleshooting > > suggest

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

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Awesome! Thanks! Please indicate in the title that this is an [NFC]. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110810/new/ https://review

[PATCH] D110910: [analyzer][solver] Fix CmpOpTable handling bug

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

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

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

<    10   11   12   13   14   15   16   17   18   19   >