Re: [PATCH] D20853: [clang-tidy] misc-macro-parentheses: Don't insert parentheses in variable declarations. Fixes bugzilla 26273

2016-06-03 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good with a few style comments. Comment at: clang-tidy/misc/MacroParenthesesCheck.cpp:82 @@ +81,3 @@ + + // If we see int/short/struct etc just assume this is a variabl

Re: [PATCH] D20689: [clang-tidy] Suspicious Call Argument checker

2016-06-03 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. In http://reviews.llvm.org/D20689#443266, @varjujan wrote: > Yes, I did. The results from running the checker on LLVM are in the attached > file. Sadly, I could'nt find any real mistakes but as I wrote in the summary, > false positives can still indicate bad naming conve

Re: [PATCH] D18575: [clang-tidy] New checker to replace deprecated throw() specifications

2016-06-03 Thread Alexander Kornienko via cfe-commits
alexfh resigned from this revision. alexfh removed a reviewer: alexfh. alexfh added a comment. Cleaning up my Phab dashboard. If http://reviews.llvm.org/D20428 doesn't happen and we'll have to return to this implementation, please re-add me to the reviewers. http://reviews.llvm.org/D18575 _

Re: [PATCH] D20196: [clang-tidy] Inefficient string operation

2016-06-03 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:31 @@ +30,3 @@ +MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus) return; + clang-format

Re: [PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

2016-06-03 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Removing from my dashboard until http://reviews.llvm.org/D20428 is submitted. http://reviews.llvm.org/D20693 ___ cfe-commits mailing li

Re: [PATCH] D19165: [clang-tidy] Add modernize-increment-bool check.

2016-06-03 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. > -Wdeprecated-increment-bool does it. But what I see from SemaExpr.cpp in > CheckIncrementDecrementOperand, it doesn't have any fixits. If an automated fix for this issue makes sen

Re: [PATCH] D18821: Add bugprone-bool-to-integer-conversion

2016-06-03 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: docs/clang-tidy/checks/bugprone-bool-to-integer-conversion.rst:37 @@ +36,3 @@ + +It turns out that the common bug is to have function returning only bools but having int as retur

Re: [PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

2016-06-03 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. Most of the fixits are still useless, but I didn't notice any false positives (i.e. all warnings would be useful to some degree). So I suggest disabling the fixes completely at least for now. Comment at: include/llvm-c/Core.h:604 @@ -603,3 +603,3 @@ *

Re: [PATCH] D20857: [clang-tidy] Add modernize-explicit-operator-bool check.

2016-06-03 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. Looks like a useful check to have. I'm not sure though, that it has anything to do with "modernize". I'd suggest adding a new "bugprone" module (should be added by http://reviews.llvm.org/D18821, hopefully soon) and moving the check there. Comment at:

Re: [PATCH] D21059: [clang-tidy] Ignore the deleted function in misc-definitions-in-headers.

2016-06-07 Thread Alexander Kornienko 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-definitions-in-headers-cxx11.hpp:6 @@ +5,3 @@ + +int f() = delete; +// CHECK-NOT: [misc-definitions-in-headers] -

Re: [PATCH] D21050: [clang-tidy] correct clang-tidy-diff.py help message

2016-06-07 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good. Thank you for fixing the copy-paste failures! Do you need me to commit the patch for you? Comment at: clang-tidy/tool/clang-tidy-diff.py:42 @@ -41,3 +41,3 @@

[clang-tools-extra] r271992 - [clang-tidy] readability-identifier-naming - Support for Type Aliases

2016-06-07 Thread Alexander Kornienko via cfe-commits
Author: alexfh Date: Tue Jun 7 04:11:19 2016 New Revision: 271992 URL: http://llvm.org/viewvc/llvm-project?rev=271992&view=rev Log: [clang-tidy] readability-identifier-naming - Support for Type Aliases Summary: Added support for Type Alias declarations. Reviewers: alexfh Subscribers: cfe-commi

Re: [PATCH] D20856: [clang-tidy] readability-identifier-naming - Support for Type Aliases

2016-06-07 Thread Alexander Kornienko via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL271992: [clang-tidy] readability-identifier-naming - Support for Type Aliases (authored by alexfh). Changed prior to commit: http://reviews.llvm.org/D20856?vs=59189&id=59847#toc Repository: rL LLVM

[clang-tools-extra] r272144 - [clang-tidy] correct clang-tidy-diff.py help message

2016-06-08 Thread Alexander Kornienko via cfe-commits
Author: alexfh Date: Wed Jun 8 09:27:43 2016 New Revision: 272144 URL: http://llvm.org/viewvc/llvm-project?rev=272144&view=rev Log: [clang-tidy] correct clang-tidy-diff.py help message Summary: Looks like the original code was copied from clang-format-diff.py. Update help message to make it clan

Re: [PATCH] D21050: [clang-tidy] correct clang-tidy-diff.py help message

2016-06-08 Thread Alexander Kornienko via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL272144: [clang-tidy] correct clang-tidy-diff.py help message (authored by alexfh). Changed prior to commit: http://reviews.llvm.org/D21050?vs=59813&id=60035#toc Repository: rL LLVM http://reviews.ll

Re: [PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

2016-06-08 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:142 @@ +141,3 @@ +SourceManager &SM(PP->getSourceManager()); +if (!SM.isInMainFile(MacroNameTok.getLocation())) + return; I'm not sure the check currently refus

[clang-tools-extra] r272155 - [clang-tidy] misc-argument-comment: don't bail out when an argument is a macro expansion (e.g. NULL).

2016-06-08 Thread Alexander Kornienko via cfe-commits
Author: alexfh Date: Wed Jun 8 10:27:46 2016 New Revision: 272155 URL: http://llvm.org/viewvc/llvm-project?rev=272155&view=rev Log: [clang-tidy] misc-argument-comment: don't bail out when an argument is a macro expansion (e.g. NULL). Add CHECK-FIX tests. Modified: clang-tools-extra/trunk/c

Re: [PATCH] D21223: [clang-tidy] misc-move-const-arg: Detect if result of std::move() is being passed as a const ref argument

2016-06-13 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. Thank you for working on this! A few more comments in addition to what Aaron has written. Comment at: clang-tidy/misc/MoveConstantArgumentCheck.cpp:22 @@ +21,3 @@ + +void ReplaceCallWithArg(const CallExpr *TheCallExpr, DiagnosticBuilder *Diag, +

Re: [PATCH] D21243: Fix clang-tidy patterns to adapt to newly added ExprWithCleanups nodes.

2016-06-13 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. Does this just fix the tests broken with http://reviews.llvm.org/D20498 or have you looked at other checks that might be broken by http://reviews.llvm.org/D20498, but don't have relevant tests? In the latter case we'd certainly need your help figuring out where further p

Re: [PATCH] D21243: Fix clang-tidy patterns to adapt to newly added ExprWithCleanups nodes.

2016-06-13 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. In http://reviews.llvm.org/D21243#455999, @alexfh wrote: > Does this just fix the tests broken with http://reviews.llvm.org/D20498 or > have you looked at other checks that might be broken by > http://reviews.llvm.org/D20498, but don't have relevant tests? In the latter

Re: [PATCH] D20196: [clang-tidy] Inefficient string operation

2016-06-13 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:41 @@ +40,3 @@ + + const auto PlusOperatorMatcher = + cxxOperatorCallExpr( s/Matcher// =

Re: [PATCH] D21223: [clang-tidy] misc-move-const-arg: Detect if result of std::move() is being passed as a const ref argument

2016-06-16 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. LG. Thanks! http://reviews.llvm.org/D21223 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21223: [clang-tidy] misc-move-const-arg: Detect if result of std::move() is being passed as a const ref argument

2016-06-16 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. BTW, I'll commit the patch for you. http://reviews.llvm.org/D21223 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21366: [clang-tidy] misc-move-const-arg: Fix typos

2016-06-16 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. I'll commit the patch for you. http://reviews.llvm.org/D21366 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

[clang-tools-extra] r272897 - [clang-tidy] misc-move-const-arg: Fix typos

2016-06-16 Thread Alexander Kornienko via cfe-commits
Author: alexfh Date: Thu Jun 16 09:32:54 2016 New Revision: 272897 URL: http://llvm.org/viewvc/llvm-project?rev=272897&view=rev Log: [clang-tidy] misc-move-const-arg: Fix typos Reviewers: alexfh Subscribers: cfe-commits Patch by Martin Boehme! Differential Revision: http://reviews.llvm.org/D21

Re: [PATCH] D21223: [clang-tidy] misc-move-const-arg: Detect if result of std::move() is being passed as a const ref argument

2016-06-16 Thread Alexander Kornienko via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL272896: [clang-tidy] misc-move-const-arg: Detect if result of std::move() is being… (authored by alexfh). Changed prior to commit: http://reviews.llvm.org/D21223?vs=60686&id=60974#toc Repository: rL

Re: [PATCH] D21366: [clang-tidy] misc-move-const-arg: Fix typos

2016-06-16 Thread Alexander Kornienko via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL272897: [clang-tidy] misc-move-const-arg: Fix typos (authored by alexfh). Changed prior to commit: http://reviews.llvm.org/D21366?vs=60802&id=60975#toc Repository: rL LLVM http://reviews.llvm.org/D2

[clang-tools-extra] r272896 - [clang-tidy] misc-move-const-arg: Detect if result of std::move() is being passed as a const ref argument

2016-06-16 Thread Alexander Kornienko via cfe-commits
Author: alexfh Date: Thu Jun 16 09:32:41 2016 New Revision: 272896 URL: http://llvm.org/viewvc/llvm-project?rev=272896&view=rev Log: [clang-tidy] misc-move-const-arg: Detect if result of std::move() is being passed as a const ref argument Summary: Conceptually, this is very close to the existing

Re: [PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

2016-06-16 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good with a couple of nits. Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:808 @@ +807,3 @@ + addUsage(NamingCheckFailures, ID, Range); + return; +}

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

2016-06-16 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. The check seems reasonable, I'm surprised there's no warning in Clang that catches index[array] syntax ;) A few comments re: implementation. Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:43 @@ +42,3 @@ + +static StringRef getAsString(c

Re: [PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

2016-06-17 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. In http://reviews.llvm.org/D21020#460734, @JamesReynolds wrote: > Thanks! All fixed now. Could you land this for me please? Sure, will do. Thank you for working on this! http://reviews.llvm.org/D21020 ___ cfe-commits maili

[clang-tools-extra] r272993 - [clang-tidy] readability-identifier-naming - Support for Macros

2016-06-17 Thread Alexander Kornienko via cfe-commits
Author: alexfh Date: Fri Jun 17 04:25:24 2016 New Revision: 272993 URL: http://llvm.org/viewvc/llvm-project?rev=272993&view=rev Log: [clang-tidy] readability-identifier-naming - Support for Macros Summary: Added support for macro definitions. -- 1. Added a pre-processor callback to catch macro d

Re: [PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

2016-06-17 Thread Alexander Kornienko via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL272993: [clang-tidy] readability-identifier-naming - Support for Macros (authored by alexfh). Changed prior to commit: http://reviews.llvm.org/D21020?vs=61071&id=61074#toc Repository: rL LLVM http:/

[clang-tools-extra] r272994 - [clang-tidy] Fix doxygen errors. NFC.

2016-06-17 Thread Alexander Kornienko via cfe-commits
Author: alexfh Date: Fri Jun 17 06:43:33 2016 New Revision: 272994 URL: http://llvm.org/viewvc/llvm-project?rev=272994&view=rev Log: [clang-tidy] Fix doxygen errors. NFC. Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.h clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedHeader

[clang-tools-extra] r272995 - [clang-tidy] More doc fixes. NFC.

2016-06-17 Thread Alexander Kornienko via cfe-commits
Author: alexfh Date: Fri Jun 17 07:01:15 2016 New Revision: 272995 URL: http://llvm.org/viewvc/llvm-project?rev=272995&view=rev Log: [clang-tidy] More doc fixes. NFC. Modified: clang-tools-extra/trunk/clang-tidy/utils/IncludeSorter.h clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.h

Re: [PATCH] D20277: [clang-tidy] UnnecessaryValueParamCheck - suggest std::move() if non-const value parameter can be moved.

2016-06-17 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. A few nits. Otherwise looks good, if Sam has no objections. Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:34 @@ -29,1 +33,3 @@ +template bool isSetDifferenceEmpty(const S &S1, const S &S2) { + for (const auto &E : S1) -

Re: [PATCH] D21303: [clang-tidy] Adds performance-returning-type check.

2016-06-17 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Apart from the other reviewers' comments, I have a few concerns: 1. The "performance-returning-type" name is missing an important part of information. Maybe "performance-return-value

Re: [PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

2016-06-17 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. Removing from my dashboard until http://reviews.llvm.org/D20428 is submitted. http://reviews.llvm.org/D20693 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

2016-06-17 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Removing from my dashboard until http://reviews.llvm.org/D20428 is submitted. Repository: rL LLVM http://reviews.llvm.org/D19201 ___

Re: [PATCH] D21243: Fix clang-tidy patterns to adapt to newly added ExprWithCleanups nodes.

2016-06-17 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Yes, you're probably not the right person to fix our test coverage. Thanks for fixing the stuff that broke and I hope we'll figure out what else needs to be fixed in a finite time after the ch

Re: [PATCH] D21470: [clang-tidy] Don't run misc-definitions-in-headers check in failing TUs.

2016-06-24 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG http://reviews.llvm.org/D21470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

Re: [PATCH] D21392: [clang-tidy] Enhance redundant-expression check

2016-06-25 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. A few nits. Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:135 @@ +134,3 @@ + llvm::APSInt ValueLHS_plus1; + if (((OpcodeLHS == BO_LE && OpcodeRHS == BO_LT) || + (OpcodeLHS == BO_GT && OpcodeRHS == BO_GE)) && How about r

Re: [PATCH] D21642: [clang-tidy] boost-use-to-string arg expr location bugfix

2016-06-26 Thread Alexander Kornienko 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: test/clang-tidy/boost-use-to-string.cpp:154 @@ +153,3 @@ + float floating; + Fields* wierd; + const int &getConstInteger() const {retu

Re: [PATCH] D21747: [clang-tidy] Warning enum unused using declarations.

2016-06-27 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. LG http://reviews.llvm.org/D21747 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21642: [clang-tidy] boost-use-to-string arg expr location bugfix

2016-06-28 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments. Comment at: test/clang-tidy/boost-use-to-string.cpp:154 @@ +153,3 @@ + float floating; + Fields* wierd; + const int &getConstInteger() const {return integer;} alexfh wrote: > "wierd" is weird ;) I should have been more clear in the

Re: [PATCH] D21392: [clang-tidy] Enhance redundant-expression check

2016-07-04 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. A couple of nits. Otherwise looks good. Thanks! Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:232 @@ +231,3 @@ + +static bool rangeSubsumeRange(BinaryOperatorKind O

Re: [PATCH] D21936: [clang-tidy] UnnecessaryValueParamCheck - only warn for virtual methods

2016-07-04 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG after addressing the comment. Thanks! Comment at: test/clang-tidy/performance-unnecessary-value-param.cpp:234 @@ +233,3 @@ +}; + +struct PositiveNonVirualMethod { -

Re: [PATCH] D21472: [clang-tidy] readability-identifier-naming - support for other case types

2016-07-04 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. IIUC, Stroustrup style mandates Single_capital_letter_to_start_a_type_name (http://www.stroustrup.com/Programming/PPP-style.pdf). So both styles you're adding in this patch should not be named after Stroustrup. Also, do these come from real-life projects/requirements or

Re: [PATCH] D21815: [clang-tidy] Add 'included from' details to warning message.

2016-07-04 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/google/GlobalNamesInHeadersCheck.cpp:49 @@ -48,1 +48,3 @@ +namespace { +std::string getIncludeFromMessage(SourceLocation DeclLoc, Static functions are preferred in LLVM. http://llvm.org/docs/CodingStandards.h

Re: [PATCH] D21833: [clang-tidy] Fix more enum declaration cases in misc-unused-using-decls check.

2016-07-04 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG http://reviews.llvm.org/D21833 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

2016-07-04 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. Please re-generate the diff with full-context (http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface). A bunch of style comments. Comment at: clang-tidy/misc/MpiTypeMismatchCheck.cpp:86 @@ +85,3 @@ + +void MpiTypeMismatchCheck:

Re: [PATCH] D20196: [clang-tidy] Inefficient string operation

2016-07-04 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/performance/InefficientStringConcatenationCheck.cpp:50 @@ +49,3 @@ + hasOverloadedOperatorName("="), hasDescendant(BasicStringPlusOperator), + allOf(hasArgument( +0, allOf(declRefExpr(BasicStringType),

Re: [PATCH] D17990: [clang-tidy] minor improvements in modernise-deprecated-headers check

2016-07-04 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/modernize/DeprecatedHeadersCheck.cpp:99 @@ -88,3 +98,3 @@ // // 1. Insert std prefix for every such symbol occurance. // 2. Insert `using namespace std;` to the beginning of TU. nit: "occurrence" =

Re: [PATCH] D21472: [clang-tidy] readability-identifier-naming - support for other case types

2016-07-05 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. In http://reviews.llvm.org/D21472#473813, @JamesReynolds wrote: > Ah, I took this from a single example in the CPP core guidelines - that PDF > is indeed a very different style. An idea we toyed with was "UpperSeparated" > and "UpperSeparatedBack"? Would that work? You

Re: [PATCH] D17990: [clang-tidy] minor improvements in modernise-deprecated-headers check

2016-07-06 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments. Comment at: docs/clang-tidy/checks/modernize-deprecated-headers.rst:39 @@ +38,3 @@ +* `` -> `` +* `` -> `` // deprecated since C++11 +* `` -> `` alexfh wrote: > Not sure if "deprecated" is the right word here. The wording seems to

Re: [PATCH] D19586: Misleading Indentation check

2016-07-06 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. Gergely, it seems that the last diff is missing clang-tidy/readability/MisleadingIndentationCheck.cpp. A few more comments below. Comment at: docs/clang-tidy/checks/readability-misleading-indentation.rst:13 @@ +12,3 @@ + +The way to avoid dangling else

Re: [PATCH] D22069: clang-tidy modernize-loop-convert: preserve type of alias declaration (bug 28341)

2016-07-06 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. Thank you for fixing the bug! One comment below. Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:525 @@ +524,3 @@ + DeclarationType = DeclarationType.getNonReferenceType(); +if (Descriptor.ElemType.isNull() || DeclarationType.isNull() || +

Re: [PATCH] D22046: [clang-tidy] Add dependency on clang-headers

2016-07-06 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG http://reviews.llvm.org/D22046 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments. Comment at: clang-rename/tool/clang-rename.py:12 @@ +11,3 @@ + +map ,cf :pyf /clang-include-fixer.py + ioeric wrote: > Maybe a different key binding so that it doesn't conflict with > include-fixer's suggested key binding? `,cr`

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments. Comment at: clang-rename/tool/ClangRename.cpp:39 @@ -38,3 +38,2 @@ #include "llvm/Support/Host.h" -#include #include bkramer wrote: > omtcyf0 wrote: > > bkramer wrote: > > > This looks unrelated. > > Right, but I'm not sure one-li

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. LG http://reviews.llvm.org/D22087 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments. Comment at: clang-rename/tool/clang-rename.py:17 @@ +16,3 @@ +All you have to do now is to place a cursor on a variable/function/class which +you would like to rename and press ',cf'. You will be promted a new name if the +cursor points to a valid sym

Re: [PATCH] D22069: clang-tidy modernize-loop-convert: preserve type of alias declaration (bug 28341)

2016-07-07 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:525 @@ +524,3 @@ + DeclarationType = DeclarationType.getNonReferenceType(); +if (Descriptor.ElemType.isNull() || DeclarationType

Re: [PATCH] D21992: [clang-tidy] new cppcoreguidelines-slicing

2016-07-07 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. Thanks for the new awesome check! Please run the check on LLVM and include your analysis of the results in the patch description. Another couple of comments below. Comment at: clang-tidy/cppcoreguidelines/SlicingCheck.cpp:93 @@ +92,3 @@ + continue;

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

2016-07-07 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: clang-tidy/misc/MpiTypeMismatchCheck.cpp:147 @@ +146,3 @@ +const Type *MpiTypeMismatchCheck::argumentType(const CallExpr *const CE, +

Re: [PATCH] D18821: Add bugprone-bool-to-integer-conversion

2016-07-07 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. Sorry for the delay. Your patch was lost in my inbox. Feel free to ping me earlier. Comment at: docs/clang-tidy/checks/bugprone-bool-to-integer-conversion.rst:37 @@ +36,3 @@ + +It turns out that the common bug is to have function returning only bools but

Re: [PATCH] D20857: [clang-tidy] Add modernize-explicit-operator-bool check.

2016-07-07 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Sorry for the delay. Feel free to ping me earlier. Comment at: clang-tidy/modernize/ExplicitOperatorBoolCheck.cpp:38 @@ +37,3 @@ + Finder->addMatcher( + cxxCon

Re: [PATCH] D21799: [ASTMatchers] Add missing forEachArgumentWithParam() to code sample

2016-07-08 Thread Alexander Kornienko via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL274835: [ASTMatchers] Add missing forEachArgumentWithParam() to code sample (authored by alexfh). Changed prior to commit: http://reviews.llvm.org/D21799?vs=62103&id=63196#toc Repository: rL LLVM ht

Re: [PATCH] D21895: CFGBuilder: Fix crash when visiting a range-based for over a dependent type

2016-07-08 Thread Alexander Kornienko via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL274834: CFGBuilder: Fix crash when visiting a range-based for over a dependent type (authored by alexfh). Changed prior to commit: http://reviews.llvm.org/D21895?vs=62374&id=63194#toc Repository: rL

r274835 - [ASTMatchers] Add missing forEachArgumentWithParam() to code sample

2016-07-08 Thread Alexander Kornienko via cfe-commits
Author: alexfh Date: Fri Jul 8 05:51:00 2016 New Revision: 274835 URL: http://llvm.org/viewvc/llvm-project?rev=274835&view=rev Log: [ASTMatchers] Add missing forEachArgumentWithParam() to code sample Reviewers: klimek Subscribers: cfe-commits, klimek Patch by Martin Boehme! Differential Revis

r274834 - CFGBuilder: Fix crash when visiting a range-based for over a dependent type

2016-07-08 Thread Alexander Kornienko via cfe-commits
Author: alexfh Date: Fri Jul 8 05:50:51 2016 New Revision: 274834 URL: http://llvm.org/viewvc/llvm-project?rev=274834&view=rev Log: CFGBuilder: Fix crash when visiting a range-based for over a dependent type Summary: CFG generation is expected to fail in this case, but it should not crash. Also

Re: [PATCH] D22154: [clang-tidy] Pass absolute path to OptionsProvider::getOptions/getRawOptions.

2016-07-08 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG http://reviews.llvm.org/D22154 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

Re: [PATCH] D22188: clang-tidy/readability-identifier-naming: crash on DependentTemplateSpecializationType

2016-07-09 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thanks! Is it related to https://llvm.org/bugs/show_bug.cgi?id=28481 ? http://reviews.llvm.org/D22188 ___ cfe-commits mailing list cfe-commi

Re: [PATCH] D22091: [clang-rename] exit code-related bugfix and code cleanup

2016-07-09 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments. Comment at: clang-rename/USRLocFinder.cpp:63 @@ -62,3 +62,3 @@ const ASTContext &Context = ConstructorDecl->getASTContext(); -for (clang::CXXConstructorDecl::init_const_iterator it = ConstructorDecl->init_begin(); it != ConstructorDecl->init

Re: [PATCH] D22091: [clang-rename] exit code-related bugfix and code cleanup

2016-07-14 Thread Alexander Kornienko 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: clang-rename/USRLocFinder.cpp:73-76 @@ -73,3 +72,6 @@ SourceLocation Location = Initializer->getSourceLocation(); - String

Re: [PATCH] D22091: [clang-rename] exit code-related bugfix and code cleanup

2016-07-14 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. In http://reviews.llvm.org/D22091#483834, @omtcyf0 wrote: > Thanks, @alexfh! > > Can you please land it? It's not easy to do from the phone ;) Please ask someone else (Ben?). http://reviews.llvm.org/D22091 ___ cfe-commits

Re: [PATCH] D17586: Add a new check, readability-redundant-string-init, that checks unnecessary string initializations.

2016-02-27 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. In http://reviews.llvm.org/D17586#363466, @Eugene.Zelenko wrote: > Sorry for arriving late to party :-) > > Thank you for great check! I found a quite lot of such problems in my code > base, but I'd like to make suggestion for further improvements: PR26756. The full lin

r262138 - Workaround doxygen bug https://bugzilla.gnome.org/show_bug.cgi?id=506243

2016-02-27 Thread Alexander Kornienko via cfe-commits
Author: alexfh Date: Sat Feb 27 08:02:08 2016 New Revision: 262138 URL: http://llvm.org/viewvc/llvm-project?rev=262138&view=rev Log: Workaround doxygen bug https://bugzilla.gnome.org/show_bug.cgi?id=506243 Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst cfe/trunk/include/clang/Format

Re: [PATCH] D17756: [clang-tidy] Make 'modernize-pass-by-value' fix work on header files.

2016-03-01 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thanks! Comment at: test/clang-tidy/modernize-pass-by-value-header.cpp:1 @@ +1,2 @@ +// RUN: cp %S/Inputs/modernize-pass-by-value/header.h %T/pass-by-value-header.h +// RU

Re: [clang-tools-extra] r262257 - IdentifierNamingCheck.cpp: try to fix MSVC build

2016-03-01 Thread Alexander Kornienko via cfe-commits
Thanks for the fix! When did it start failing? The last change in this file was quite a while ago. Did it start breaking because of a change in a header? On Mon, Feb 29, 2016 at 10:17 PM, Hans Wennborg via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: hans > Date: Mon Feb 29 15:17:39

Re: SourceTypeInfo for long long

2016-03-01 Thread Alexander Kornienko via cfe-commits
+cfe-commits for wider audience. Hi Piotr, On Tue, Mar 1, 2016 at 2:36 PM, Piotr Padlewski wrote: > Hi Alexey, > I am currently writing new check > https://llvm.org/bugs/show_bug.cgi?id=26763 > Awesome! Sorry I didn't have time yet to comment on the issues you filed, though I have a couple of

Re: SourceTypeInfo for long long

2016-03-01 Thread Alexander Kornienko via cfe-commits
Answered on the review thread. The problem is with BuiltinTypeLoc not storing the location for the end token of builtin type names consisting of multiple tokens. This can be worked around by re-lexing a fragment of the code for now. Richard, does it make sense to store the end location in BuiltinT

Re: [PATCH] D17491: Add performance check to flag function parameters of expensive to copy types that can be safely converted to const references.

2016-03-01 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:25-32 @@ +24,10 @@ +std::string paramNameOrIndex(StringRef Name, size_t Index) { + std::string Output; + llvm::raw_string_ostream Stream(Output); + if (!Name.empty()) { +Stream <

Re: [PATCH] D17772: [clang-tidy]: string_view of temporary string

2016-03-01 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. As David noted, there are other similar classes like `llvm::StringRef`, `llvm::ArraryRef`, `gsl::span`, `StringPiece` (in various Google projects, e.g. https://github.com/google/re2/blob/master/re2/stringpiece.h) and numerous other in different code bases. I think, we sh

Re: [PATCH] D17488: Extend UnnecessaryCopyInitialization check to trigger on non-const copies that can be safely converted to const references.

2016-03-01 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good. Thanks! http://reviews.llvm.org/D17488 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

Re: [PATCH] D17488: Extend UnnecessaryCopyInitialization check to trigger on non-const copies that can be safely converted to const references.

2016-03-01 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/utils/FixItHintUtils.cpp:2 @@ +1,3 @@ +//===--- FixItHintUtils.cpp - +// clang-tidy---===// +// Please fix the line break. http://reviews.llvm.org/D17488

Re: [clang-tools-extra] r261738 - [clang-tidy] introduce modernize-deprecated-headers check

2016-03-01 Thread Alexander Kornienko via cfe-commits
Kirill, FYI. On Thu, Feb 25, 2016 at 3:21 AM, Richard Smith wrote: > On Wed, Feb 24, 2016 at 5:36 AM, Alexander Kornienko via cfe-commits > wrote: > > Author: alexfh > > Date: Wed Feb 24 07:36:34 2016 > > New Revision: 261738 > > > > URL: http://llvm.org/vie

Re: [PATCH] D17805: [clang-tidy] Fix an assertion failure of "SLocEntry::getExpansion()" when IncludeInserter handles macro header file.

2016-03-02 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good! Thank you for finding and fixing this! Comment at: clang-tidy/utils/IncludeInserter.cpp:23 @@ -21,3 +22,3 @@ void InclusionDirective(SourceLocation HashLocation

r262603 - [docs] Updated doxygen files to work well with doxygen 1.8.11

2016-03-03 Thread Alexander Kornienko via cfe-commits
Author: alexfh Date: Thu Mar 3 04:42:46 2016 New Revision: 262603 URL: http://llvm.org/viewvc/llvm-project?rev=262603&view=rev Log: [docs] Updated doxygen files to work well with doxygen 1.8.11 Doxygen 1.8.11 doesn't seem to like files with ".intro" extension by default. Removed: cfe/trunk/

r262604 - [docs] Add missing file

2016-03-03 Thread Alexander Kornienko via cfe-commits
Author: alexfh Date: Thu Mar 3 04:44:10 2016 New Revision: 262604 URL: http://llvm.org/viewvc/llvm-project?rev=262604&view=rev Log: [docs] Add missing file Added: cfe/trunk/docs/doxygen-mainpage.dox Added: cfe/trunk/docs/doxygen-mainpage.dox URL: http://llvm.org/viewvc/llvm-project/cfe/tru

[clang-tools-extra] r262605 - [docs] Fix docs to work with doxygen 1.8.11

2016-03-03 Thread Alexander Kornienko via cfe-commits
Author: alexfh Date: Thu Mar 3 04:45:59 2016 New Revision: 262605 URL: http://llvm.org/viewvc/llvm-project?rev=262605&view=rev Log: [docs] Fix docs to work with doxygen 1.8.11 Added: clang-tools-extra/trunk/docs/doxygen-mainpage.dox Removed: clang-tools-extra/trunk/docs/doxygen.intro Mod

Re: [clang-tools-extra] r262615 - [clang-tidy] Do not emit warnings from misc-suspicious-semicolon when the compilation fails.

2016-03-03 Thread Alexander Kornienko via cfe-commits
On Thu, Mar 3, 2016 at 2:08 PM, Gabor Horvath via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: xazax > Date: Thu Mar 3 07:08:11 2016 > New Revision: 262615 > > URL: http://llvm.org/viewvc/llvm-project?rev=262615&view=rev > Log: > [clang-tidy] Do not emit warnings from misc-suspiciou

Re: [PATCH] D17849: [clang-tidy] Make 'modernize-use-nullptr' check work on multiple nested implicit cast expressions.

2016-03-03 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rL LLVM http://reviews.llvm.org/D17849 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/ma

Re: [clang-tools-extra] r262615 - [clang-tidy] Do not emit warnings from misc-suspicious-semicolon when the compilation fails.

2016-03-03 Thread Alexander Kornienko via cfe-commits
Thank you! On Thu, Mar 3, 2016 at 2:48 PM, Gábor Horváth wrote: > Thank you for the suggestions. This should be addressed in r262618. > > On 3 March 2016 at 14:42, Alexander Kornienko wrote: > >> >> >> On Thu, Mar 3, 2016 at 2:08 PM, Gabor Horvath via cfe-commits < >> cfe-commits@lists.llvm.org

[clang-tools-extra] r262787 - [docs] Clean up doxygen comments a bit.

2016-03-05 Thread Alexander Kornienko via cfe-commits
Author: alexfh Date: Sat Mar 5 22:05:59 2016 New Revision: 262787 URL: http://llvm.org/viewvc/llvm-project?rev=262787&view=rev Log: [docs] Clean up doxygen comments a bit. Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.h Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.h URL:

Re: [PATCH] D17811: [clang-tidy] Add check to detect dangling references in value handlers.

2016-03-07 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/misc/DanglingHandleCheck.cpp:24 @@ +23,3 @@ + +std::vector parseClasses(StringRef Option) { + SmallVector Classes; A very similar code has been added recently to clang-tidy/utils/HeaderFileExtensionsUtils.*. M

Re: [PATCH] D17926: [clang-tidy] Don't delete unused parameter in class override method in anonymous namespace.

2016-03-07 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/misc/UnusedParametersCheck.cpp:20 @@ +19,3 @@ +namespace { +bool isOverrideMethod(const FunctionDecl *Function) { + if (const auto *MD = dyn_cast(Function)) There's a matcher for this: `isOverride`. Repositor

Re: [PATCH] D17926: [clang-tidy] Don't delete unused parameter in class override method in anonymous namespace.

2016-03-07 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/misc/UnusedParametersCheck.cpp:80 @@ +79,3 @@ + UsedByRef() || + !ast_matchers::match(cxxMethodDecl(isOverride()), *Function, + *Result.Context) I meant, you can use it inside

Re: [PATCH] D17811: [clang-tidy] Add check to detect dangling references in value handlers.

2016-03-10 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thank you for one more awesome check! Comment at: clang-tidy/misc/DanglingHandleCheck.cpp:25 @@ +24,3 @@ +std::vector parseClasses(StringRef Option) { + SmallVector Class

Re: [PATCH] D17482: [clang-tidy] Allow tests to verify changes made to header files

2016-03-10 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. Sorry for leaving this without attention for a while. I'm somewhat concerned about this change. It's adding a certain level of complexity, but doesn't cover some less trivial cases like handling of multiple headers. Can you take a look at existing tests and say how many

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