[PATCH] D65578: [analyzer][NFC] Make sure that the BugReport is not modified during the construction of non-visitor pieces

2019-08-01 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 212811. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65578/new/ https://reviews.llvm.org/D65578 Files: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h

[PATCH] D65650: [Analyzer] Iterator Checkers - Fix for Crash on Iterator Differences

2019-08-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. Cool! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65650/new/ https://reviews.llvm.org/D65650 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D62525: [Analyzer] Add new visitor to the iterator checkers

2019-08-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1495-1496 // For deque-like containers invalidate all iterator positions. For // vector-like containers invalidate iterator positions after the insertion. const auto *Cont = Po

[PATCH] D65723: [analyzer][NFC] Add different interestingness kinds

2019-08-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, xazax.hun, Charusso, dcoughlin, baloghadamsoftware, rnkovacs. Szelethus added a project: clang. Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity. Szelethus added pa

[PATCH] D65724: [analyzer] Don't make ConditionBRVisitor events prunable when the condition is an interesting field

2019-08-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, xazax.hun, Charusso, baloghadamsoftware, rnkovacs, dcoughlin. Szelethus added a project: clang. Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity. Szelethus added a

[PATCH] D65725: [analyzer] Mention whether an event is about a condition in a bug report part 2

2019-08-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, xazax.hun, baloghadamsoftware, Charusso, rnkovacs, dcoughlin. Szelethus added a project: clang. Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity. Szelethus added a

[PATCH] D65484: [analyzer][NFC] Refactoring BugReporter.cpp P5.: Compact mile long function invocations into objects

2019-08-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:111 +/// getters and some well placed asserts for extra security. +class BugReportConstruct { + /// The consumer we're constructing the bug repo

[PATCH] D64374: [analyzer] CastValueChecker: Model casts

2019-08-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Do you have plans to extend this checker with the modeling of `isa<>`? I might take that off your shoulder if not :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64374/new/ https://reviews.llvm.org/D64374 ___ cf

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

2019-08-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Cheers! :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.org/D65889 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi

[PATCH] D65725: [analyzer] Mention whether an event is about a condition in a bug report part 2

2019-08-08 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: clang/test/Analysis/track-control-dependency-conditions.cpp:465 void assert(int b) { - if (!b) // tracking-note{{Assuming 'b' is not equal to 0}} + if (!b) // tracking-note{{Assuming 'b' i

[PATCH] D65287: [analyzer][CFG] Don't track the condition of asserts

2019-08-08 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked 2 inline comments as done. Szelethus added a comment. After testing this patch out, I'm confident it works pretty well. Take a look at the following two runs: 138 notes

[PATCH] D65575: [analyzer] Mention whether an event is about a condition in a bug report part 1

2019-08-08 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D65575#1611013 , @NoQ wrote: > Fantastic! Let's open the wording bikeshed season? > > I suspect that a simple "(The) Value -> Condition value" change would have > worked better. > > Another variant: "Value ..., which particip

[PATCH] D65287: [analyzer][CFG] Don't track the condition of asserts

2019-08-08 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: clang/test/Analysis/track-control-dependency-conditions.cpp:469-471 +extern void __assert_fail (__const char *__assertion, __const char *__file, + unsigned int __lin

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-10 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. There is a discussion to be had about the core package. So @NoQ suggested that we could hide the entire thing just as we do with apiModeling, but I argued that the users wouldn't be exposed to the checker descriptions. However, it makes little sense to caution our use

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-10 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D66042#1624365 , @Charusso wrote: > In D66042#1624320 , @Szelethus wrote: > > > I think it would make a lot more sense to create a separate (and hidden!) > > coreModeling package that

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-10 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D66042#1624462 , @Charusso wrote: > My solution preserve the checkers as they are and yours definitely would > rewrite them. Checker writing has tons of boilerplates, I think adding more > should be avoided. Why would you to

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-10 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D66042#1624468 , @Szelethus wrote: > Speaking about performance impact, note where your patch does the actual > silencing: by the time control reaches that point, we created bug report > equivalence classes, constructed a tr

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-10 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D66042#1624469 , @Charusso wrote: > In D66042#1624468 , @Szelethus wrote: > > > Speaking about performance impact, note where your patch does the actual > > silencing: by the time cont

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-10 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. In D66042#1624478 , @Charusso wrote: > In D66042#1624475 , @Szelethus wrote: > > > In D66042#16

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

2019-08-11 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added reviewers: gamesh411, NoQ. Szelethus added subscribers: gamesh411, NoQ. Szelethus added a comment. + @gamesh411 as he took over this checker before I commited it on his behalf, +@NoQ because he is far more knowledgeable about this part of the analyzer. Comment

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-12 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D66042#1624697 , @Charusso wrote: > In D66042#1624684 , @NoQ wrote: > > > My idea here was that this new feature isn't going to be user-facing. We > > aren't promising to support all c

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

2019-08-12 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. In D64274#1623644 , @NoQ wrote: > In D64274#1600834 , @Szelethus wrote: > > > In D64274#1598226

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

2019-08-12 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Oh, btw, thank you for working on this! Comment at: clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:121-122 // Check if any of the enum values possibly match. bool PossibleValueMatch = llvm::any_of( DeclValues, Constraint

[PATCH] D65575: [analyzer] Mention whether an event is about a condition in a bug report part 1

2019-08-12 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. A little early, gentle ping :) I'm getting kinda paranoid with the size of the stack, and how much I struggled with commiting last time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65575/new/ https://reviews.llvm.org/D

[PATCH] D65575: [analyzer] Mention whether an event is about a condition in a bug report part 1

2019-08-12 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D65575#1625741 , @NoQ wrote: > In D65575#1625738 , @Szelethus wrote: > > > A little early, gentle ping :) I'm getting kinda paranoid with the size of > > the stack, and how much I stru

[PATCH] D65287: [analyzer][CFG] Don't track the condition of asserts

2019-08-12 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 214708. Szelethus marked 12 inline comments as done. Szelethus added a comment. Addressing reviewer feedback! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65287/new/ https://reviews.llvm.org/D65287 Files: clang/include/clang/Analysis/CFG.h c

[PATCH] D65287: [analyzer][CFG] Don't track the condition of asserts

2019-08-12 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:5634 + // immediately caught. + if (std::any_of(Blk->begin(), Blk->end(), [](const CFGElement &Elm) { +if (Optional StmtElm = Elm.getAs()) xazax.hun wrote: > I am wondering, should

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Too bad we're in different time zones, I can only respond in a giant comment, but I wouldn't want to make anyone feel left out :) The conclusion to my responses is this: - I recently sent out a mail to cfe-dev (I hope to have added your correct email adresses!) that

[PATCH] D66131: [analyzer] Don't track the condition of foreach loops

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, xazax.hun, baloghadamsoftware, dcoughlin, rnkovacs, Charusso. Szelethus added a project: clang. Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity. As discussed on t

[PATCH] D65378: [analyzer][NFC] Refactoring BugReporter.cpp P1.: Store interesting symbols/regions in a simple set

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL368689: [analyzer][NFC] Refactoring BugReporter.cpp P1.: Store interesting… (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D65379: [analyzer][NFC] Refactoring BugReporter.cpp P2.: Clean up the construction of bug paths and finding a valid report

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL368694: [analyzer][NFC] Refactoring BugReporter.cpp P2.: Clean up the construction of… (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Change

[PATCH] D66136: Remove no-op callbacks from TemplateInstantiationCallback

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. They are used by Templight, which we are planning to eventually move to clang-tools extra, though our resources are a bit limited at the time :) https://github.com/mikael-s-per

[PATCH] D65381: [analyzer][NFC] Refactoring BugReporter.cpp P3.: std::shared_pointer -> PathDiagnosticPieceRef

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL368717: [analyzer][NFC] Refactoring BugReporter.cpp P3.: std… (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: http

[PATCH] D66152: Fix false negatives of statement local lifetime analysis for some STL implementation

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Apologies for going off-topic, but would it make sense to start lifetime related patches with the tag [LifetimeAnalysis] or similar (like [analyzer] for static analyzer patches)? I dont feel knowledgable enough to participate, but would gladly add a herald rule to be

[PATCH] D65382: [analyzer][NFC] Refactoring BugReporter.cpp P4.: If it can be const, make it const

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL368735: [analyzer][NFC] Refactoring BugReporter.cpp P4.: If it can be const, make it… (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed

[PATCH] D65484: [analyzer][NFC] Refactoring BugReporter.cpp P5.: Compact mile long function invocations into objects

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL368737: [analyzer][NFC] Refactoring BugReporter.cpp P5.: Compact mile long function… (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed

[PATCH] D65484: [analyzer][NFC] Refactoring BugReporter.cpp P5.: Compact mile long function invocations into objects

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus reopened this revision. Szelethus added a comment. This revision is now accepted and ready to land. Oh, crap, forgot to address inlines. I'll see how buildbots react to these changes and will follow up in a smaller patch. Reopening as a reminder. Repository: rL LLVM CHANGES SINCE L

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D66042#1627765 , @NoQ wrote: > In D66042#1626631 , @Szelethus wrote: > > > In D66042#1626513 , @Charusso > > wrote: > > > > > I really appreaci

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. We might also want to change the revision title, but the commit message for sure, to make it clear that this affects all checkers, something along the lines of "[analyzer] Add an analyzer config to silence diagnostics from user specified checkers/packages" CHANGES S

[PATCH] D65484: [analyzer][NFC] Refactoring BugReporter.cpp P5.: Compact mile long function invocations into objects

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus closed this revision. Szelethus marked 5 inline comments as done. Szelethus added a comment. The deed is done. Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:111 +/// getters and some well placed asserts for extra security. +class BugReportConstruct { + //

[PATCH] D61027: Fix crash on switch conditions of non-integer types in templates

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D61027#1627785 , @gribozavr wrote: > Like @riccibruno said, `check-clang-tools` will run them. However, before > committing a patch, please run `check-all` -- you never know what your patch > can affect. And also you'll ne

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. LGTM, thanks! Do you need someone to commit this on your behalf? Also, could you please make the comments capitalized, terminated, and fitting in 80 columns? Comment at: clang/test/Analysis/enum-cast-out-of-range.c:1

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D66042#1627842 , @Charusso wrote: > Any analyzer config flag is equally accessible to anyone as the driver flags > as they are both flags. The only difference is the config flags are more code > to implement, and a lot more

[PATCH] D65487: [analyzer][NFC] Refactoring BugReporter.cpp P6.: Completely get rid of interestingness propagation

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL368752: [analyzer][NFC] Refactoring BugReporter.cpp P6.: Completely get rid of… (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior

[PATCH] D65578: [analyzer][NFC] Make sure that the BugReport is not modified during the construction of non-visitor pieces

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL368755: [analyzer][NFC] Make sure that the BugReport is not modified during the… (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prio

[PATCH] D64232: [analyzer] Prune calls to functions with linear CFGs that return a non-zero constrained value

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Szelethus marked 5 inline comments as done. Closed by commit rL368771: [analyzer] Prune calls to functions with linear CFGs that return a non-zero… (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald

[PATCH] D64287: [analyzer] Track the right hand side of the last store regardless of its value

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Szelethus marked 2 inline comments as done. Closed by commit rG0df9c8c57802: [analyzer] Track the right hand side of the last store regardless of its value (authored by Szelethus). Changed prior to commit: https://reviews

[PATCH] D65287: [analyzer][CFG] Don't track the condition of asserts

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. If you have no major objections, could you please accept formally? :) And I'll promise to draw CFG graphs all day tomorrow as an exercise after reading the order of operator evaluation rules >.< CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65287/new/ https:

[PATCH] D64270: [analyzer][NFC] Prepare visitors for different tracking kinds

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL368777: [analyzer][NFC] Prepare visitors for different tracking kinds (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit

[PATCH] D66192: [analyzer] Don't delete TaintConfig copy constructor

2019-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. This really shouldn't matter much. Go for it! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66192/new/ https://reviews.llvm.org/D66192

[PATCH] D64272: [analyzer] Note last writes to a condition only in a nested stackframe

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL368817: [analyzer] Note last writes to a condition only in a nested stackframe (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D66042#1629019 , @alexfh wrote: > In D66042#1627760 , @NoQ wrote: > > > In D66042#1627193 , @alexfh wrote: > > > > > But without this patch clan

[PATCH] D65287: [analyzer][CFG] Don't track the condition of asserts

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL368836: [analyzer][CFG] Don't track the condition of asserts (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https

[PATCH] D65182: [analyzer] Add fix-it hint support.

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added subscribers: gribozavr, aaron.ballman, alexfh. Szelethus added a comment. Hmm, why the need for checker options? Why not have them by default? If fixits are an experimental feature, maybe we should have a global `enable-fixits` config maybe. But

[PATCH] D65290: [analyzer][NFC] Prove that we only track the evaluated part of the condition

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL368853: [analyzer][NFC] Prove that we only track the evaluated part of the condition (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed

[PATCH] D66042: [analyzer] Analysis: Silence checkers

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Thanks!!! Please note that BugReporter.cpp (especially the parts you touched) just got refactored extensively, so you'll need to rebase on master. Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:192 /// Pair of checker name a

[PATCH] D66042: [analyzer] Analysis: Silence checkers

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def:392 +StringRef, RawSilencedCheckers, "silence-checkers", +"A semicolon separated list of checker and package names to silence.", "") + Hmm, maybe we c

[PATCH] D66042: [analyzer] Analysis: Silence checkers

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:468-502 void CheckerRegistry::validateCheckerOptions() const { for (const auto &Config : AnOpts.Config) { StringRef SuppliedChecker; StringRef SuppliedOption; s

[PATCH] D65196: [CFG] Introduce CFGElementRef, a wrapper that knows it's position in a CFGBlock

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Szelethus marked an inline comment as done. Closed by commit rL368883: [CFG] Introduce CFGElementRef, a wrapper that knows it's position in a CFGBlock (authored by Szelethus, committed by ). Herald added a project: LLVM. Her

[PATCH] D66042: [analyzer] Analysis: Silence checkers

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Now that I had ever more time to think about this patch, I see a great potential in it for development use, for example, we could silence a checker before splitting it up to see whether we could disable it altogether or really having to go through the process of split

[PATCH] D66042: [analyzer] Analysis: Silence checkers

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I swear this is my last objection :) As soon as this is settled, I'll accept. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:483-504 + if (!AnOpts.RawSilencedCheckersAndPackages.empty()) { +std::vector Checkers = +AnOpts.getRegiste

[PATCH] D66042: [analyzer] Analysis: Silence checkers

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:483-504 + if (!AnOpts.RawSilencedCheckersAndPackages.empty()) { +std::vector Checkers = +AnOpts.getRegisteredCheckers(/*IncludeExperimental=*/true); +std::vector Packages = +

[PATCH] D66042: [analyzer] Analysis: Silence checkers

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:483-504 + if (!AnOpts.RawSilencedCheckersAndPackages.empty()) { +std::vector Checkers = +AnOpts.getRegisteredCheckers(/*IncludeExperimental=*/true); +std::vector Packages = +

[PATCH] D66042: [analyzer] Analysis: Silence checkers

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. You know what, I argued that we should use configs because this flag is incomplete. So I shouldn't be all up and down that it isn't. LGTM! CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D66042: [analyzer] Analysis: Silence checkers

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:483-504 + if (!AnOpts.RawSilencedCheckersAndPackages.empty()) { +std::vector Checkers = +AnOpts.getRegisteredCheckers(/*IncludeExperimental=*/true); +std::vector Packages = +

[PATCH] D65724: [analyzer] Don't make ConditionBRVisitor events prunable when the condition is an interesting field

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 215195. Szelethus marked an inline comment as done. Szelethus added a comment. - Make the generic messages in-class, `constexpr` fields. - Provide plenty of test cases for captured lambda variables. In our meeting we had both had a sight of relief, and some

[PATCH] D65724: [analyzer] Don't make ConditionBRVisitor events prunable when the condition is an interesting field

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked 4 inline comments as done. Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:192-194 + if (const auto *DRE = dyn_cast(CondVarExpr)) +if (const auto *VD = dyn_cast(DRE->getDecl())) + return State->getSVal(

[PATCH] D65724: [analyzer] Don't make ConditionBRVisitor events prunable when the condition is an interesting field

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked 2 inline comments as done. Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:192-194 + if (const auto *DRE = dyn_cast(CondVarExpr)) +if (const auto *VD = dyn_cast(DRE->getDecl())) + return State->getSVal(

[PATCH] D65065: [clang-tidy] Possibility of displaying duplicate warnings

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I have no authority to accept patches in clang-tidy (though please feel free to add me as a reviewer, I can more easily participate in the discussion!), this looks good to me too, thanks! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://revie

[PATCH] D66049: [analyzer] PR41729: Fix some false positives and improve strlcat and strlcpy modeling

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I agree with @gamesh411 here, would it be much trouble to see how this behaves on a bigger codebase? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66049/new/ https://reviews.llvm.org/D66049 __

[PATCH] D66042: [analyzer] Analysis: Silence checkers

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:483-504 + if (!AnOpts.RawSilencedCheckersAndPackages.empty()) { +std::vector Checkers = +AnOpts.getRegisteredCheckers(/*IncludeExperimental=*/true); +std::vector Packages = +

[PATCH] D66042: [analyzer] Analysis: Silence checkers

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:483-504 + if (!AnOpts.RawSilencedCheckersAndPackages.empty()) { +std::vector Checkers = +AnOpts.getRegisteredCheckers(/*IncludeExperimental=*/true); +std::vector Packages = +

[PATCH] D65182: [analyzer] Add fix-it hint support.

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h:879 + ArrayRef getFixits() const { return Fixits; } + NoQ wrote: > Szelethus wrote: > > Hmm, will this return an immutable container? If not, can

[PATCH] D65361: [analyzer] Trust global initializers when analyzing main().

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I really like the high level idea proposed by this patch, and the test files make me believe that its correct as well! I'm really not familiar around this part of the code, so if its okay, I'll take my time to do the usual find references, inserting unreachables/asser

[PATCH] D60281: [analyzer] Add docs for cplusplus.InnerPointer

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus commandeered this revision. Szelethus edited reviewers, added: rnkovacs; removed: Szelethus. Szelethus added a comment. This revision now requires review to proceed. I'll gladly add the finishing touches :) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D6

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

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D63279#1606063 , @baloghadamsoftware wrote: > For me it is all the same whether to continue this patch or taking over > Péter's. Well, there are plenty of comments I'm not sure ever got addressed. We could take an invento

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

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D63279#1630491 , @NoQ wrote: > the only reason it's not enabled by default yet is because we forgot to do > that :( Damn, thats a bummer. It wouldn't be too much trouble to run an analysis and check out whether everythin

[PATCH] D65182: [analyzer] Add fix-it hint support.

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. In D65182#1630540 , @NoQ wrote: > In D65182#1629192 , @Szelethus wrote: > > > Hmm, why the need for checker options? Why not have them by default? If > >

[PATCH] D60281: [analyzer] Add docs for cplusplus.InnerPointer

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 215281. Szelethus changed the repository for this revision from rC Clang to rG LLVM Github Monorepo. Szelethus added a comment. Revised the documentation according to @NoQ's comments. By literally copy pasting it. Like any good programmer should do :^) R

[PATCH] D66261: [analyzer] Warn about -analyzer-configs being meant for development purposes only

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, xazax.hun, baloghadamsoftware, Charusso, rnkovacs, dcoughlin, dkrupp. Szelethus added a project: clang. Herald added subscribers: cfe-commits, gamesh411, donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity. This is more of a

[PATCH] D60281: [analyzer] Add docs for cplusplus.InnerPointer

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 215288. Szelethus marked 2 inline comments as done. Szelethus added a comment. Fixed! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60281/new/ https://reviews.llvm.org/D60281 Files: clang/docs/analyzer/checkers.rst Index: clang/docs/analyzer/

[PATCH] D60281: [analyzer] Add docs for cplusplus.InnerPointer

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D60281#1630646 , @NoQ wrote: > Thanks! > > One more quick question: Who is Husi??? Well thats me. Long story short, the reason why my name is Szelethus is that my first ever character to hit max level in World of Warcraft w

[PATCH] D66265: [NFCI] Always initialize BugReport const fields

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus 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/D66265/new/ https://reviews.llvm.org/D66265

[PATCH] D66267: [analyzer] TrackConstraintBRVisitor: Do not track unknown values

2019-08-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Shouldn't we just delete this entire visitor altogether and merge it into ConditionBRVisitor (like, eventually, not right now)? It seems to be a relic of the past. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66267/new/ https://revie

[PATCH] D60281: [analyzer] Add docs for cplusplus.InnerPointer

2019-08-15 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL368979: [analyzer] Add docs for cplusplus.InnerPointer (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://rev

[PATCH] D66261: [analyzer] Warn about -analyzer-configs being meant for development purposes only

2019-08-15 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL368980: [analyzer] Warn about -analyzer-configs being meant for development purposes… (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

2019-08-15 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Yup, you can upload it and the green checkmark will stay. If it doesn't, I'll accept again. I think there was one case when I added like 800 extra LOCs to a patch and phabricator automatically marked it as "needs reviews", but I never came across this after that, eve

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

2019-08-15 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Oh, there is no need for a new differential, you can update this one by clicking on 'update diff' in the right panel. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66014/new/ https://reviews.llvm.org/D66014 _

[PATCH] D66267: [analyzer] TrackConstraintBRVisitor: Do not track unknown values

2019-08-15 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D66267#1632164 , @NoQ wrote: > In D66267#1630728 , @Szelethus wrote: > > > Shouldn't we just delete this entire visitor altogether and merge it into > > ConditionBRVisitor (like, event

[PATCH] D66267: [analyzer] TrackConstraintBRVisitor: Do not track unknown values

2019-08-15 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1898-1900 + // If the contents are symbolic and null, find out when they became null. + if (V.getAsLocSymbol(/*IncludeBaseRegions=*/true)) +if (LVState->isNull(V).

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

2019-08-15 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. You seem to have diffed against your latest local commit, rather than against trunk (or master, if you use the monorepo). Phabricator isn't smart enough to put two and two together, and only displays the uploaded diff (though one has to admit, its doing a damn good jo

[PATCH] D66333: [analyzer] NonNullParamChecker and CStringChecker parameter number in checker message

2019-08-16 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I really like the change, thanks! Let's settle on the diagnostic message then. Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:202 + SVal l, + unsigned idx = -1u) con

[PATCH] D65453: [analyzer] Improve the accuracy of the Clang call graph analysis

2019-08-16 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I hope you dont mind me changing the revision title -- many of us are subscribed to the "analyzer" tag and like learning about whats changing :) This is not an objection to the patch or anything of course, just adding some lurkers. CHANGES SINCE LAST ACTION https:

[PATCH] D65287: [analyzer][CFG] Don't track the condition of asserts

2019-08-17 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1759 +if (isa(ElseCond)) { + assert(cast(ElseCond)->isLogicalOp()); + return isAssertlikeBlock(Else, Context);

[PATCH] D66381: [analyzer] Enable control dependency condition tracking by default

2019-08-17 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, xazax.hun, dcoughlin, Charusso, baloghadamsoftware, rnkovacs. Szelethus added a project: clang. Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity. Szelethus added a

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

2019-08-18 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. Thank you, and sorry for dragging you through this! At the very least, we got to learn a lot from it :) Comment at: clang/include/clang/StaticAnalyzer/Core/CheckerManager.h:93-96 // This wrapper is used to ensure th

[PATCH] D62899: [analyzer][UninitializedObjectChecker] Mark uninitialized regions as interesting.

2019-08-18 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D62899#1551312 , @NoQ wrote: > In D62899#1549715 , @Szelethus wrote: > > > Added a proper testfile. The only downside of it is that it doesn't test > > anything. > > > Use creduce! I

[PATCH] D66267: [analyzer] TrackConstraintBRVisitor: Do not track unknown values

2019-08-19 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. In D66267#1632344 , @Charusso wrote: > I would upstream my hotfix of nullability without any tests as the comment > says the intention and also we have plenty of tests of > `TrackConstraintBR

[PATCH] D66423: [analyzer] CastValueChecker: Model isa(), isa_and_nonnull()

2019-08-19 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Whoa, the test cases look AMAZING! I admit that I haven't read much of the other code just yet, but I like the results in any case. Comment at: clang/test/Analysis/cast-value-notes.cpp:5-27 namespace llvm { template const X *cast(Y Value); tem

[PATCH] D66460: [analyzer] Remove BugReporter.BugTypes.

2019-08-19 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. > So we'll most likely make a clear separation between a basic bug report(er) > and a path-sensitive bug report(er), having them inherit from common bug > report(er) classes. That sound

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