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

2019-08-07 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In D65065#1617031 , @gribozavr wrote: > > This suggestion would result another strange behavior: if the user disables > > cert-err09-cpp because he or she doesn't want to see its reports, the other > > one (cert-err61-cpp) will st

[PATCH] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

2019-08-12 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. One more nit. Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:674 + return !( + (isTokAtEndOfExpr(Lsr, LTok, SM) && isTokAtEndOfExpr(Rsr, RTok, SM)) && + isSameToken(LTok, RTok, SM)); aaron.ballman wrote: > You can dr

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

2019-08-12 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In D66042#1624081 , @NoQ wrote: > +@alexfh because clang-tidy now finally has a way of safely disabling core > checkers without causing crashes all over the place! Would you like to take > the same approach as we picked in scan-bu

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

2019-08-13 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In D66042#1625898 , @NoQ wrote: > While we're here: i poked your silencing mechanism a little bit and even > though i'm still pretty sure you couldn't have done it perfectly without our > help, it sounds as if the only problem you

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

2019-08-14 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In D66042#1627760 , @NoQ wrote: > In D66042#1627193 , @alexfh wrote: > > > But without this patch clang seems to have the same two ANALYZE log lines > > regardless of whether I enable `core`

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

2019-08-16 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. Just FYI, https://bugs.llvm.org/show_bug.cgi?id=43019 is relevant. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65065/new/ https://reviews.llvm.org/D65065 ___ cfe-commits mailin

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

2019-08-16 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thanks! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65065/new/ https://reviews.llvm.org/D65065 _

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

2019-08-20 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: clang-tools-extra/test/clang-tidy/bugprone-dynamic-static-initializers.hpp:1 +// RUN: %check_clang_tidy %s bugprone-dynamic-static-initializers %t + aaron.ballman wrote: > I'm a bit confused. > > 1) Why is this file a .

[PATCH] D62125: Run ClangTidy tests in all C++ language modes

2019-05-20 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. That's a quite impressive amount of work. Thanks! Looks good. In D62125#1508113 , @JonasToth wrote: > Wow, a lot of work! > > I did not check all test

[PATCH] D55765: Fix use-after-free bug in Tooling.

2018-12-17 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: include/clang/Tooling/Tooling.h:208 std::unique_ptr -buildASTFromCode(const Twine &Code, const Twine &FileName = "input.cc", +buildASTFromCode(StringRef Code, const Twine &FileName = "input.cc", std::shared_ptr PCHConta

[PATCH] D55765: Fix use-after-free bug in Tooling.

2018-12-18 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55765/new/ https://reviews.llvm.org/D55765 ___ cfe-commits mail

[PATCH] D54945: This commit adds a chapter about clang-tidy integrations

2018-12-18 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: docs/clang-tidy/index.rst:262 + +Clang-tidy integrated +- So how about moving this chapter to a separate page and adding a link to it at the top of this document? While certainly useful, the informati

[PATCH] D54945: This commit adds a chapter about clang-tidy integrations

2019-01-07 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In D54945#1340528 , @MarinaKalashina wrote: > @alexfh Just for me to be sure, should there be the following structure in > http://clang.llvm.org/extra/index.html: ... Roughly like that. More specifically, I'd suggest to modify t

[PATCH] D56012: [clang-tidy] Be more liberal about literal zeroes in abseil checks

2019-01-07 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. One random late comment. Comment at: clang-tools-extra/trunk/test/clang-tidy/abseil-duration-factory-scale.cpp:38 + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale] + // CHE

[PATCH] D55765: Fix use-after-free bug in Tooling.

2019-01-08 Thread Alexander Kornienko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL350638: Fix use-after-free bug in Tooling. (authored by alexfh, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55765/

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

2019-09-17 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. A couple of drive-by comments. Comment at: clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:22-33 +constexpr llvm::StringLiteral FuncExprName = "entire-called-function-expr"; +constexpr llvm::StringLiteral CastExprName = "cast-expr

[PATCH] D67501: [clang-tidy] Fix relative path in header-filter.

2019-09-23 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In D67501#1678962 , @gribozavr wrote: > Sorry, I reverted this patch in r372601. > > Unfortunately, it makes paths printed in clang-tidy'd diagnostics > inconsistent with what `-header-filter` operates on. > > For example, imagine

[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

2018-04-12 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D44602#1065331, @lebedev.ri wrote: > @alexfh i'm waiting for some comment from you. there was no comment added > when you removed yourself as a reviewer, > so i *assume* the usual message of that time was meant - "removing from my > review qu

[PATCH] D45392: [clang-tidy] add new check to find out objc ivars which do not have prefix '_'

2018-04-12 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: test/clang-tidy/readability-identifier-naming-objc.m:8-9 +// RUN: ]}' -- -fno-delayed-template-parsing \ +// RUN: -I%S/Inputs/readability-identifier-naming \ +// RUN: -isystem %S/Inputs/readability-identifier-naming/system + ---

[PATCH] D45392: [clang-tidy] add new check to find out objc ivars which do not have prefix '_'

2018-04-12 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. BTW, it looks like the http://clang.llvm.org/extra/clang-tidy/checks/objc-property-declaration.html check could also be replaced with readability-identifier-naming (not in this patch). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45392 ___

[PATCH] D45697: [clang-tidy] Fix clang-tidy doesn't read .clangtidy configuration file.

2018-04-17 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Thank you for investigating and fixing this! One comment re: the test below. Comment at: test/clang-tidy/read_file_config.cpp:5 +// RUN: echo '[{"command": "cc -c

[PATCH] D45718: Allow registering custom statically-linked analyzer checkers

2018-04-17 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh created this revision. alexfh added a reviewer: george.karpenkov. Herald added a subscriber: a.sidorin. Add an extension point to allow registration of statically-linked Clang Static Analyzer checkers that are not a part of the Clang tree. This extension point employs the mechanism used whe

[PATCH] D45697: [clang-tidy] Fix clang-tidy doesn't read .clangtidy configuration file.

2018-04-17 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added inline comments. This revision is now accepted and ready to land. Comment at: test/clang-tidy/read_file_config.cpp:5 +// RUN: echo '[{"command": "cc -c -o test.o test.cpp", "directory": "%T/read-file-config", "file": "%T/read-file-conf

[PATCH] D45776: [clang-tidy] Customize FileCheck prefix in check_clang-tidy.py

2018-04-18 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. Thank you for the contribution! Please update the documentation accordingly (http://clang.llvm.org/extra/clang-tidy/#testing-checks). > // RUN: %check_clang_tidy %s misc-unused-using-decls %t > -check_suffix=-FLAG_1-- I don't know whether it makes sense to endorse (or

[PATCH] D45392: [clang-tidy] add new check to find out objc ivars which do not have prefix '_'

2018-04-20 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thanks! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45392 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://

[PATCH] D45160: [clang-apply-replacements] Make clang-apply-replacements installable

2018-04-20 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D45776: [clang-tidy] Customize FileCheck prefix in check_clang-tidy.py

2018-04-20 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. A few style-related comments. Comment at: docs/clang-tidy/index.rst:677 +To check more than one scenario in the same test file use +``-check-suffix=SUFFIX_NAME`` on ``check_clang_tidy.py`` command line. +With ``-check-suffix=SUFFIX_NAME`` you need to re

[PATCH] D45912: update test to use ivar in implementation instead of class extension

2018-04-20 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D45776: [clang-tidy] Customize FileCheck prefix in check_clang-tidy.py

2018-04-21 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D45776 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D45718: [analyzer] Allow registering custom statically-linked analyzer checkers

2018-04-22 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D45718#1074611, @george.karpenkov wrote: > Actually, we do have unittests in `tools/clang/unittest/Analysis`. > @alexfh would you be able to write a unittest in there? It should be fairly > easy following the structure of e.g. > `tools/clang/

[PATCH] D46003: [clang-tidy] Fix PR35468

2018-04-24 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG with one comment. Comment at: test/clang-tidy/misc-unconventional-assign-operator.cpp:1 -// RUN: %check_clang_tidy %s misc-unconventional-assign-operator %t -- -- -std=c+

[PATCH] D46146: [analyzer] pr37152: Fix operator delete[] array-type-sub-expression handling.

2018-04-26 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:1089-1091 +// Yes, it may even be a multi-dimensional array. +while (const auto *AT = getContext().getAsArrayType(DTy)) + DTy = AT->getElementType(); Maybe add a FIXME t

[PATCH] D45927: [clang-tidy] [modernize-use-auto] Correct way to calculate a type name length for multi-token types

2018-04-30 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/modernize/UseAutoCheck.cpp:33 + +bool IsNotSpace(const char& C) { + return !std::isspace(static_cast(C)); Why `const ch

[PATCH] D45927: [clang-tidy] [modernize-use-auto] Correct way to calculate a type name length for multi-token types

2018-04-30 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. Actually, just ignoring spaces may be not the best option. In https://reviews.llvm.org/D45927#1074593, @zinovy.nis wrote: > > I think spaces that will be removed should be counted - long long is 9. > > I thought about it, but what about "long long int >

[PATCH] D46233: [ASTMatchers] Overload isConstexpr for ifStmts

2018-04-30 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: docs/LibASTMatchersReference.html:179 -MatcherDecl>cxxMethodDeclMatcher

[PATCH] D46233: [ASTMatchers] Overload isConstexpr for ifStmts

2018-04-30 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: docs/LibASTMatchersReference.html:179 -MatcherDecl>cxxMethodDeclMatcherCXXMethodDecl>... +Matcher

[PATCH] D45927: [clang-tidy] [modernize-use-auto] Correct way to calculate a type name length for multi-token types

2018-04-30 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D45927#1083051, @zinovy.nis wrote: > > I think, it's 13, if you choose to remove stars, and 17 otherwise. The > > difference is excessive spaces vs. required ones. Implementing proper logic > > may be involved, but we can simplify it to someth

[PATCH] D45931: [ASTMatchers] Don't garble the profiling output when multiple TU's are processed

2018-04-30 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. From a user's perspective I'd probably prefer a different behavior of checks profiling with multiple translation units: per-file table after each file and an aggregate table at the end. An independent improvement could be to support TSV/CSV output and/or dumping to a fil

[PATCH] D46293: [clang-tidy/google-runtime-int] Allow passing non-bitwidth types to printf()-style APIs

2018-04-30 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG with one comment. Thank you for the fix! Comment at: test/clang-tidy/google-runtime-int.cpp:77 +__attribute__((__format__ (__printf__, 1, 2))) +void myprintf(const char* s

[PATCH] D46293: [clang-tidy/google-runtime-int] Allow passing non-bitwidth types to printf()-style APIs

2018-04-30 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/google/IntegerTypesCheck.cpp:66 + Finder->addMatcher(typeLoc(loc(isInteger()), + unless(hasAncestor(callExpr( + callee(functionDecl(hasAttr(attr::Format))) -

[PATCH] D45932: [clang-tidy][modernize-raw-string-literal] Don't replace upper ASCII with raw literals

2018-05-02 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: test/clang-tidy/modernize-raw-string-literal.cpp:42 char const *const HexNonPrintable("\\\x03"); char const *const Delete("\\\177"); +char const *const MultibyteSnowman("\xE2\x98\x83"); zinovy.nis wrote: > By the way, A

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

2018-05-03 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. As Devin says (and as we discussed this with Anna Zaks) alpha checkers are still in development, so we don't want to expose them to the users, even very curious ones. For those who want to help with development of the alpha checkers, there's always a possibility to chang

[PATCH] D46393: Remove explicit cfg-temporary-dtors=true

2018-05-03 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh created this revision. alexfh added a reviewer: NoQ. Remove explicit -analyzer-config cfg-temporary-dtors=true in analyzer tests, since this option defaults to true since r326461. Repository: rC Clang https://reviews.llvm.org/D46393 Files: test/Analysis/cfg-rich-constructors.cpp t

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

2018-05-03 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/ClangTidy.cpp:373-376 // FIXME: Remove this option once clang's cfg-temporary-dtors option defaults // to true. AnalyzerOptions-

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

2018-05-03 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D46159#1086332, @lebedev.ri wrote: > In https://reviews.llvm.org/D46159#1086322, @alexfh wrote: > > > > > > How about `-enable-alpha-checks=yes-i-know-they-are-broken` ? A hidden flag with a scary name without any way to specify it in .clang-t

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

2018-05-03 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D46159#1086479, @pfultz2 wrote: > > But still, could you explain the use case and why a local modification of > > clang-tidy is not an option? > > Because I would like to direct users to try an alpha check on their local > codebases without ha

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

2018-05-03 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D46159#1086487, @alexfh wrote: > In https://reviews.llvm.org/D46159#1086479, @pfultz2 wrote: > > > > But still, could you explain the use case and why a local modification of > > > clang-tidy is not an option? > > > > Because I would like to di

[PATCH] D45927: [clang-tidy] [modernize-use-auto] Correct way to calculate a type name length for multi-token types

2018-05-03 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/modernize-use-auto-min-type-name-length.cpp:61-83 +long int li = static_cast(foo()); +// CHECK-FIXES-0-0: auto li = {{.*}} +/

[PATCH] D46325: [clang-tidy] Define __clang_analyzer__ macro for clang-tidy for compatibility with clang static analyzer. 2nd try.

2018-05-03 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/ClangTidy.cpp:536 + DiagnosticConsumer *DiagConsumer) override { + Invocation->getFrontendOpts().ProgramAction

[PATCH] D40937: [clang-tidy] Infinite loop checker

2018-05-03 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/bugprone/InfiniteLoopCheck.h:37 +private: + bool updateSequence(Stmt *FunctionBody, ASTContext &ASTCtx); + const Stmt *PrevFunctionBody

[PATCH] D33844: [clang-tidy] terminating continue check

2018-05-03 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. There are still outstanding comments. https://reviews.llvm.org/D33844 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://li

[PATCH] D33537: [clang-tidy] Exception Escape Checker

2018-05-03 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. It looks like you've missed some comments or uploaded a wrong patch. Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:105 +const TypeVec throwsException(con

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

2018-05-03 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. In https://reviews.llvm.org/D46159#1086493, @aaron.ballman wrote: > I think the premise is a bit off the mark. It's not that these are not for > the common user -- it's that they're

[PATCH] D46187: [Analyzer] getRegisteredCheckers(): handle debug checkers too.

2018-05-03 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. What's the use case for debug CSA checkers in clang-tidy? Repository: rC Clang https://reviews.llvm.org/D46187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

[PATCH] D45931: [ASTMatchers] Don't garble the profiling output when multiple TU's are processed

2018-05-03 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D45931#1084503, @lebedev.ri wrote: > In https://reviews.llvm.org/D45931#1083192, @lebedev.ri wrote: > > > Thank you for looking at this. > > > > In https://reviews.llvm.org/D45931#1083184, @alexfh wrote: > > > > > From a user's perspective I'd p

[PATCH] D46325: [clang-tidy] Define __clang_analyzer__ macro for clang-tidy for compatibility with clang static analyzer. 2nd try.

2018-05-03 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thank you! https://reviews.llvm.org/D46325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D45931: [ASTMatchers] Don't garble the profiling output when multiple TU's are processed

2018-05-04 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D45931#1087007, @lebedev.ri wrote: > In https://reviews.llvm.org/D45931#1086665, @alexfh wrote: > > > In https://reviews.llvm.org/D45931#1084503, @lebedev.ri wrote: > > > > > In https://reviews.llvm.org/D45931#1083192, @lebedev.ri wrote: > > > >

[PATCH] D33537: [clang-tidy] Exception Escape Checker

2018-05-04 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Thank you for the updates. A few more comments. Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:150 + } else if (const auto *Try = dyn_cast(St)) { +au

[PATCH] D46393: Remove explicit cfg-temporary-dtors=true

2018-05-04 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D46393#1086887, @NoQ wrote: > Thanks! > > Just curious - did these flags bother you? Cause we never really care about > cleaning up run lines after flipping the flag, so we have a lot of such stale > flags in our tests. We could start cleaning

[PATCH] D46393: Remove explicit cfg-temporary-dtors=true

2018-05-04 Thread Alexander Kornienko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL331520: Remove explicit cfg-temporary-dtors=true (authored by alexfh, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D46393 Files: cfe/trunk/te

[PATCH] D40937: [clang-tidy] Infinite loop checker

2018-05-04 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Could you run the check on llvm sources and post a summary of results here? A few more inline comments. Comment at: clang-tidy/bugprone/InfiniteLoopCheck.cpp:102 +

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

2018-05-04 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D46159#1086732, @aaron.ballman wrote: > If the static analyzer people desire this feature, that would sway my > position on it, but it sounds like they're hesitant as well. > However, I don't think clang-tidy is beholden either -- if we don't

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

2018-05-08 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Given Artem's answer (and if there are no objections from other CSA maintainers), I have no concerns with this patch going in. A couple of minor nits. Comment at: clang-tid

[PATCH] D46233: [ASTMatchers] Overload isConstexpr for ifStmts

2018-05-08 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D46233 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D46504: [clang-tidy] Profile is a per-AST (per-TU) data.

2018-05-08 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG with a couple of nits. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:181 std::unique_ptr OptionsProvider) -: DiagEngine(nullptr), OptionsProvider(std::mo

[PATCH] D46188: [clang-tidy] Add a flag to enable debug checkers

2018-05-08 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. I don't think debug CSA checkers belong to clang-tidy. If one needs to dump CFG, they are probably doing quite involved stuff already, so going a step further and running `clang -cc1

[PATCH] D46188: [clang-tidy] Add a flag to enable debug checkers

2018-05-08 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D46188#1091237, @lebedev.ri wrote: > In https://reviews.llvm.org/D46188#1091221, @alexfh wrote: > > > I don't think debug CSA checkers belong to clang-tidy. If one needs to dump > > CFG, they are probably doing quite involved stuff already, so

[PATCH] D46187: [Analyzer] getRegisteredCheckers(): handle debug checkers too.

2018-05-08 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D46187#1088722, @NoQ wrote: > It seems that you're using debug checkers of the analyzer to gain some free > tools for exploring the source code (such as displaying a call graph) for > free, right? > > I believe we could also benefit from a met

[PATCH] D45679: [clang-tidy] Add ExprMutationAnalyzer, that analyzes whether an expression is mutated within a statement.

2018-06-27 Thread Alexander Kornienko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE335736: [clang-tidy] Add ExprMutationAnalyzer, that analyzes whether an expression is… (authored by alexfh, committed by ). Changed prior to commit: https://reviews.llvm.org/D45679?vs=151245&id=153087

[PATCH] D46951: [clang-tidy] misc-unused-parameters - retain old behavior under StrictMode

2018-06-27 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. Ping. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46951 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45718: [analyzer] Allow registering custom statically-linked analyzer checkers

2018-06-27 Thread Alexander Kornienko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC335740: [analyzer] Allow registering custom statically-linked analyzer checkers (authored by alexfh, committed by ). Changed prior to commit: https://reviews.llvm.org/D45718?vs=152317&id=153091#toc Rep

[PATCH] D45679: [clang-tidy] Add ExprMutationAnalyzer, that analyzes whether an expression is mutated within a statement.

2018-06-28 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp:582 + + AST = tooling::buildASTFromCode("namespace std { class type_info; }" + "void f() { int x; typeid(x = 10); }"); FYI, this test had

[PATCH] D48717: [clang-tidy] fix PR36489 - respect deduced pointer types from auto as well

2018-06-28 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp:24 + const auto AllPointerTypes = anyOf( + hasType(pointerType()), hasType(autoType(hasDeducedType(pointerType(); Can anyOf be pushed inside ha

[PATCH] D48717: [clang-tidy] fix PR36489 - respect deduced pointer types from auto as well

2018-06-28 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48717 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D48708: NFC Build fix in RegisterCustomCheckersTest.cpp

2018-06-28 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp:64 }); -return AnalysisConsumer; +return std::unique_ptr(AnalysisConsumer.relea

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

2018-06-28 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:191 +void templated_thrower() { throw T{}(); } +// CHECK-MESSAGES: [[@LINE-1]]:34: warning: throwing an exception whose type 'int' is not derived from 'std::exception' + hoke

[PATCH] D46951: [clang-tidy] misc-unused-parameters - retain old behavior under StrictMode

2018-06-28 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh updated this revision to Diff 153327. alexfh marked 3 inline comments as done. alexfh added a comment. - Addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46951 Files: clang-tidy/misc/UnusedParametersCheck.cpp clang-tidy/misc/UnusedParametersC

[PATCH] D46951: [clang-tidy] misc-unused-parameters - retain old behavior under StrictMode

2018-06-28 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/misc/UnusedParametersCheck.cpp:180 +// (constructor initializer counts for non-empty body). +if (StrictMode || llvm::distance(Function->getBody()->children()) > 0 || +(isa(Function) && lebedev.r

[PATCH] D46951: [clang-tidy] misc-unused-parameters - retain old behavior under StrictMode

2018-06-28 Thread Alexander Kornienko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL335863: [clang-tidy] misc-unused-parameters - retain old behavior under StrictMode (authored by alexfh, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm

[PATCH] D48759: [ASTMatchers] add matcher for decltypeType and its underlyingType

2018-06-29 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. Please add a test. Repository: rC Clang https://reviews.llvm.org/D48759 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

2018-07-04 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. Some patterns are covered by compiler diagnostics: https://godbolt.org/g/HvsjnP. Is there any benefit in re-implementing them? Comment at: clang-tidy/misc/IncorrectPointerCastCheck.cpp:60 +diag(CastExpr->getLocStart(), + "Do not use

[PATCH] D36892: [clang-tidy] check_clang_tidy.py: support CHECK-NOTES prefix

2018-07-11 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. As per the previous comment: I have no concerns as long as the documentation is updated and at least one existing test is changed to use this feature (see the list in the previous co

[PATCH] D33537: [clang-tidy] Exception Escape Checker

2018-07-11 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good with one comment. Comment at: test/clang-tidy/bugprone-exception-escape.cpp:178 +void indirect_implicit() noexcept { + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning:

[PATCH] D42682: [clang-tidy] Add io-functions-misused checker

2018-07-11 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. (removing from my dashboard) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42682 ___ cfe-commits mailing list cfe-com

[PATCH] D49213: [analyzer] pr38072: Suppress an assertion failure for eliding the same destructor twice due to the default argument problem.

2018-07-12 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. FTR, this is http://llvm.org/PR38072 Repository: rC Clang https://reviews.llvm.org/D49213 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68807: [ClangTidy] Separate tests for infrastructure and checkers

2019-10-11 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. I agree that the test/clang-tidy directory has become hard to navigate. Splitting tests for checks and for infrastructure seems reasonable to me. I personally don't care about specific directo

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

2019-10-14 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In D68682#1706581 , @poelmanc wrote: > Thanks, from the name that sounds like the perfect place to do it. If > `cleanupAroundReplacements` also is used by clang-format, would we want to > make the functionality optional, e.g. via

[PATCH] D69036: [libTooling] Fix r374962: add more Transformer forwarding decls.

2019-10-16 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69036/new/ https://reviews.llvm.org/D69036 ___

[PATCH] D68885: Add a Ranges field to Diagnostic struct

2019-10-18 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: include/clang/Tooling/Core/Diagnostic.h:93 + /// representation of the source code associated with the diagnostic. + ArrayRef Ranges; }; Please add YAML serialization/deserialization code for this. See include/clang/T

[PATCH] D83223: [clang-tidy] Header guard check can skip past license comment

2020-07-21 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In D83223#2147306 , @aaron.ballman wrote: > In D83223#2147247 , @njames93 wrote: > > > In D83223#2147072 , @aaron.ballman > > wrote: > > > > > > /

[PATCH] D75184: [clang-tidy] Optional inheritance of file configs from parent directories 

2020-04-13 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Apologies for the delay! It's sort of a crazy time now =\ The code looks mostly good now modulo a few comments inline. Comment at: clang-tools-extra/clang-tidy/ClangTidyChec

[PATCH] D77983: clang-tidy doc: add a note for every checker with an autofix

2020-04-13 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/abseil-duration-conversion-cast.rst:29-35 Note: In the second example, the suggested fix could yield a different result, as the conversion to integer could truncate. In practice, this is very r

[PATCH] D82089: [clang-tidy] modernize-loop-convert reverse iteration support

2020-08-10 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Thanks for the patch! Looks generally good. A few comments inline. Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:74-76 +static const char

[PATCH] D85218: In clang-tidy base checks prevent anonymous functions from triggering assertions

2020-08-10 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. LG with a couple of comments. Do you need someone to land the patch for you? Comment at: clang-tools-extra/clang-tidy/add_new_check.py:139 const auto *MatchedDecl = Result.Nodes.getNodeAs("x"); - if (MatchedDecl->getNam

[PATCH] D85666: [clang-tidy] IncludeInserter: allow <> in header name

2020-08-10 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh created this revision. alexfh added reviewers: aaron.ballman, hokein. Herald added subscribers: cfe-commits, arphaman, kbarton, xazax.hun, nemanjai. Herald added a project: clang. alexfh requested review of this revision. Herald added a subscriber: wuzish. This adds a pair of overloads for

[PATCH] D82089: [clang-tidy] modernize-loop-convert reverse iteration support

2020-08-10 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:1011-1015 + if (Header.front() == '<' && Header.back() == '>') { +IsSystemInclude = true; +Header.pop_back(); +Header.erase(0, 1); + } alexfh wrot

[PATCH] D40388: [clang-tidy] rename_check.py misc-string-constructor bugprone-string-constructor

2017-11-23 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh created this revision. Herald added subscribers: xazax.hun, mgorny. Rename misc-string-constructor to bugprone-string-constructor + manually update the lenght of '==='s in the doc file. https://reviews.llvm.org/D40388 Files: clang-tidy/bugprone/BugproneTidyModule.cpp clang-tidy/bugpr

[PATCH] D40388: [clang-tidy] rename_check.py misc-string-constructor bugprone-string-constructor

2017-11-23 Thread Alexander Kornienko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL318916: [clang-tidy] rename_check.py misc-string-constructor bugprone-string-constructor (authored by alexfh). Changed prior to commit: https://reviews.llvm.org/D40388?vs=124065&id=124070#toc Repositor

[PATCH] D40389: [clang-tidy] rename_check.py misc-dangling-handle bugprone-dangling-handle

2017-11-23 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh created this revision. Herald added subscribers: xazax.hun, mgorny. https://reviews.llvm.org/D40389 Files: clang-tidy/bugprone/BugproneTidyModule.cpp clang-tidy/bugprone/CMakeLists.txt clang-tidy/bugprone/DanglingHandleCheck.cpp clang-tidy/bugprone/DanglingHandleCheck.h clang-tid

[PATCH] D40392: [clang-tidy] rename_check.py misc-argument-comment bugprone-argument-comment

2017-11-23 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh created this revision. Herald added subscribers: xazax.hun, mgorny. + manually convert the unit test to lit test. https://reviews.llvm.org/D40392 Files: clang-tidy/bugprone/ArgumentCommentCheck.cpp clang-tidy/bugprone/ArgumentCommentCheck.h clang-tidy/bugprone/BugproneTidyModule.cp

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