Re: [PATCH] D23257: Fix clang-tidy crash when a single fix is applied on multiple files.

alexfh added inline comments. Comment at: test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h:1 @@ +1,2 @@ +#include + hokein wrote: > Usually test should not use STL headers as it relies on the system headers. > > You don't have to use std::string

Re: [PATCH] D23243: [clang-tidy] enhance modernize-use-bool-literals to check ternary operator

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG with a nit. Comment at: test/clang-tidy/modernize-use-bool-literals.cpp:124 @@ +123,3 @@ + bool Result = Value == 1 ? 1 : 0; + // CHECK-MESSAGES: :[[@LINE-1]]:30: {{.*}}

Re: [PATCH] D23193: [clang-rename] fix bug with initializer lists

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. The test is fine now. Looks good once the other comment is addressed. https://reviews.llvm.org/D23193 ___ cfe-commits mailing list cfe-commits@li

Re: [PATCH] D23158: [clang-rename] merge tests when possible

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Removing from my dashboard until the comments are addressed. https://reviews.llvm.org/D23158 ___ cfe-commits mailing list cfe-commits@l

Re: [PATCH] D23257: Fix clang-tidy crash when a single fix is applied on multiple files.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. A bunch of nits. Otherwise looks good. Thank you for the fix! Comment at: clang-tidy/ClangTidy.cpp:519 @@ -513,2 +518,3 @@ tooling::TranslationUnitReplacements TUR; for

Re: [clang-tools-extra] r277677 - [clang-tidy] Inefficient string operation

3 PM Alexander Kornienko via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: alexfh >> Date: Wed Aug 3 18:06:03 2016 >> New Revision: 277677 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=277677&view=rev >> Log: >> [clang-

Re: [PATCH] D22725: [clang-tidy] Add check 'modernize-use-algorithm'

alexfh added inline comments. Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:59-61 @@ +58,5 @@ + IncludeStyle(utils::IncludeSorter::parseIncludeStyle( + Options.get("IncludeStyle", "llvm"))) { + + for (const auto &KeyValue : + {std::make_pair("memcpy",

Re: [PATCH] D23004: [ASTMatchers] Add matchers canReferToDecl() and hasUnderlyingDecl()

alexfh accepted this revision. alexfh added a reviewer: alexfh. alexfh added a comment. LG https://reviews.llvm.org/D23004 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23265: [clang-tidy] enhance readability-else-after-return

alexfh added inline comments. Comment at: clang-tidy/readability/ElseAfterReturnCheck.cpp:45 @@ +44,3 @@ + for (const auto &BindingName : + {"return", "continue", "break", "goto", "throw"}) { +if (Result.Nodes.getNodeAs(BindingName)) { This won't work i

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

alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: clang-tidy/modernize/DeprecatedHeadersCheck.cpp:55 @@ +54,3 @@ + CStyledHeaderToCxx = + {{"assert.h", "cassert"}, +{"complex.h", "complex"}, ---

Re: [PATCH] D23265: [clang-tidy] enhance readability-else-after-return

alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: clang-tidy/readability/ElseAfterReturnCheck.cpp:25 @@ +24,3 @@ + forEach(ifStmt(hasThen(stmt(anyOf( + returnStmt().bind("return"), +

Re: [PATCH] D23198: clang-rename rename-all: support reading old/newname pairs from a YAML file

alexfh added a subscriber: alexfh. alexfh added a comment. A few late comments. Comment at: clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp:65 @@ +64,3 @@ + + RenameAllInfo() : Offset(0) {} +}; I'd use inline inititalizer instead. Co

Re: [PATCH] D23265: [clang-tidy] enhance readability-else-after-return

alexfh added inline comments. Comment at: clang-tidy/readability/ElseAfterReturnCheck.cpp:45 @@ +44,3 @@ + for (const auto &BindingName : + {"return", "continue", "break", "goto", "throw"}) { +if (Result.Nodes.getNodeAs(BindingName)) { omtcyfz wrote: >

Re: [PATCH] D23265: [clang-tidy] enhance readability-else-after-return

alexfh added inline comments. Comment at: test/clang-tidy/readability-else-after-return.cpp:33 @@ -32,1 +32,3 @@ + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return' + // CHECK-FIXES: // comment f(0); omtcyfz wrote: > alexfh wrote:

Re: [clang-tools-extra] r278198 - [Documentation] Fix spelling mistakes in docs/clang-tidy/index.rst.

Thanks! Could you fix one more nit below? On Aug 10, 2016 04:03, "Eugene Zelenko via cfe-commits" < cfe-commits@lists.llvm.org> wrote: Author: eugenezelenko Date: Tue Aug 9 20:55:51 2016 New Revision: 278198 URL: http://llvm.org/viewvc/llvm-project?rev=278198&view=rev Log: [Documentation] Fix s

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

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D17990 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

Re: [PATCH] D23158: [clang-rename] merge tests when possible

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good. Thank you! https://reviews.llvm.org/D23158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mail

Re: [PATCH] D23265: [clang-tidy] enhance readability-else-after-return

alexfh added inline comments. Comment at: clang-tidy/readability/ElseAfterReturnCheck.cpp:40 @@ +39,3 @@ + for (const auto *BindingName : {"return", "continue", "break", "throw"}) { +if (Result.Nodes.getNodeAs(BindingName)) { + ControlFlowInterruptor = BindingName; -

Re: [PATCH] D23265: [clang-tidy] enhance readability-else-after-return

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D23265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

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

alexfh added inline comments. Comment at: clang-tidy/modernize/DeprecatedHeadersCheck.cpp:83 @@ +82,3 @@ + for (const auto &Key : DeleteHeaders) { +DeleteHeadersSet.insert(Key); + } No danger of dangling references here, since `StringMap` copies keys. See `

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

alexfh added a comment. In https://reviews.llvm.org/D17990#511424, @omtcyf0 wrote: > Revert changes. Thanks! If Aaron has no objections, good to go. https://reviews.llvm.org/D17990 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://li

Re: [PATCH] D23409: Make clang-tidy work with clang-cl

alexfh added a comment. In https://reviews.llvm.org/D23409#512819, @zturner wrote: > In https://reviews.llvm.org/D23409#512745, @zturner wrote: > > > In https://reviews.llvm.org/D23409#512720, @aaron.ballman wrote: > > > > > Patch generally LGTM, though I wonder if there's a way we can add test

Re: [PATCH] D23409: Make clang-tidy work with clang-cl

alexfh added a comment. May I ask you to upload patches with full diff context next time? I know, it's not directly supported by TortoiseGit, but there are at least two other reasonable ways of generating full diffs for Phabricator: http://llvm.org/docs/Phabricator.html#requesting-a-review-via-

Re: [PATCH] D23397: [clang-rename] cleanup `auto` usages

alexfh added a comment. A couple of comments inline. Comment at: clang-rename/RenamingAction.cpp:73 @@ -72,3 +72,3 @@ // FIXME: better error handling. - auto Replace = tooling::Replacement(SourceMgr, Loc, PrevNameLen, NewName); - auto Err = FileToReplaces[Repla

Re: [PATCH] D23434: Don't allow llvm-include-order to intermingle includes from different files.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. The patch LG. In https://reviews.llvm.org/D23434#513533, @djasper wrote: > I think we should entirely drop this implementation of the check and let it > just check #includes with clang-format

Re: [PATCH] D23434: Don't allow llvm-include-order to intermingle includes from different files.

alexfh added a comment. In https://reviews.llvm.org/D23434#513839, @djasper wrote: > I think we got confused. We once had tried to write an experimental separate > check to comply with Google's style guide. If you want to fiddle around with > that, contact me, I can send you pointers. But as I

Re: [PATCH] D23353: [clang-tidy] Add check 'misc-use-after-move'

alexfh added a comment. A few initial comments. Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:492 @@ +491,3 @@ + DeclRefs->clear(); + for (CFGBlock::const_iterator I = Block->begin(), E = Block->end(); I != E; + ++I) { Any reason to avoid range-based

Re: [PATCH] D23409: Make clang-tidy work with clang-cl

alexfh added a comment. The changes in Driver LGTM (though I'd prefer someone who knows Driver to look). Could you move lib/Tooling/JSONCompilationDatabase.cpp to a separate patch? It seems like an orthogonal change. As for a clang-cl-mode test for clang-tidy, clang-tidy-run-with-database.cpp i

Re: [PATCH] D23480: Add a test for clang-tidy using the clang cl driver

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG with two nits. > In order to get this to work, the positional arguments must use > --driver-mode=cl , and NOT clang-cl It's not that important for the fixed compilation database, as long

Re: [PATCH] D23476: Allow manual specification of the binary directory in check_clang_tidy.py

alexfh added a comment. Why is this change needed? What exactly doesn't work without it? I think, lit should take care of setting paths or changing directories appropriately. https://reviews.llvm.org/D23476 ___ cfe-commits mailing list cfe-commits@

Re: [PATCH] D23455: [Tooling] Parse compilation database command lines properly on Windows

alexfh added inline comments. Comment at: lib/Tooling/JSONCompilationDatabase.cpp:119 @@ -115,1 +118,3 @@ StringRef EscapedCommandLine) { +#if defined(LLVM_ON_WIN32) + llvm::BumpPtrAllocator Alloc; As noted on D23409, this will likely break mingw compatibili

Re: [PATCH] D23471: [Documentation] Improve checks groups descriptions in clang-tidy/index.rst

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Awesome! Looks good! Repository: rL LLVM https://reviews.llvm.org/D23471 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.

Re: [PATCH] D23476: Allow manual specification of the binary directory in check_clang_tidy.py

The script was never intended to be run manually. Its interface is rather inconvenient for this. If you need to run a single test, it's easier to do using llvm-lit (`path/to/lit test/file.cpp`). But if you for some reason want to run the script directly, it should be possible to just modify PATH or

Re: [PATCH] D16764: Move incorrect-roundings to upstream.

alexfh added a comment. Nice! See a few comments inline. Comment at: clang-tidy/misc/IncorrectRoundings.cpp:38 @@ +37,3 @@ + // Match a floating point expression. + auto FloatType = expr(anyOf(hasType(qualType(asString("long double"))), + hasType(q

Re: [PATCH] D16717: [clang-tidy] Add non-constant references in function parameters check.

alexfh added a comment. Thanks! A few comments. Comment at: clang-tidy/google/NonConstReferences.cpp:54 @@ +53,3 @@ + + QualType ReferencedType = + *Result.Nodes.getNodeAs("referenced_type"); This could be `auto` to avoid duplicating the type name. ==

Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check

alexfh added inline comments. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:80 @@ +79,3 @@ +return false; + + const size_t NewLinePos = Text.find(R"(\n)"); aaron.ballman wrote: > This is why I would still prefer to block on fixing StringLiteral.

Re: [PATCH] D16259: Add clang-tidy readability-redundant-control-flow check

alexfh added a comment. Thank you for adding more test cases! Could you address one more comment in a follow-up, please? Comment at: test/clang-tidy/readability-redundant-control-flow.cpp:193 @@ +192,3 @@ +// CHECK-FIXES-NEXT: {{^return;$}} +// CHECK-FIXES-NEXT: {{^ *}$}} +

Re: [PATCH] D16717: [clang-tidy] Add non-constant references in function parameters check.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good with one nit. Thank you! Comment at: test/clang-tidy/google-runtime-references.cpp:1 @@ +1,2 @@ +// RUN: %check_clang_tidy %s google-runtime-references %t -- -- -s

[clang-tools-extra] r259530 - [clang-tidy] Add non-constant references in function parameters check.

Author: alexfh Date: Tue Feb 2 11:27:01 2016 New Revision: 259530 URL: http://llvm.org/viewvc/llvm-project?rev=259530&view=rev Log: [clang-tidy] Add non-constant references in function parameters check. Summary: This is implemented originally by Alexander Kornienko. Reviewers: alexfh Subscribe

[clang-tools-extra] r259531 - [clang-tidy] Removed unnecessary parameters in the test

Author: alexfh Date: Tue Feb 2 11:27:08 2016 New Revision: 259531 URL: http://llvm.org/viewvc/llvm-project?rev=259531&view=rev Log: [clang-tidy] Removed unnecessary parameters in the test Modified: clang-tools-extra/trunk/test/clang-tidy/google-runtime-references.cpp Modified: clang-tools-e

Re: [PATCH] D16717: [clang-tidy] Add non-constant references in function parameters check.

This revision was automatically updated to reflect the committed changes. Closed by commit rL259530: [clang-tidy] Add non-constant references in function parameters check. (authored by alexfh). Changed prior to commit: http://reviews.llvm.org/D16717?vs=46628&id=4#toc Repository: rL LLVM

Re: [PATCH] D16376: clang-tidy check: rule-of-five

alexfh added a comment. In http://reviews.llvm.org/D16376#342901, @jbcoe wrote: > I think I'll move this check to `cppcoreguidelines` and call it > `rule-of-five`. Yes, please move it to cppcoreguidelines. > https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-y

Re: [PATCH] D16376: clang-tidy check: rule-of-five

alexfh added inline comments. Comment at: clang-tidy/misc/RuleOfFiveCheck.cpp:30 @@ +29,3 @@ +.bind("copy-ctor")), + unless(hasDescendant(cxxMethodDecl(isCopyAssignmentOperator(), + this); Why is `hasDe

Re: [PATCH] D16113: [clang-tdiy] Add header file extension configuration support.

alexfh added a comment. Sorry for the delay. A few more comments. Comment at: clang-tidy/google/GlobalNamesInHeadersCheck.cpp:30 @@ +29,3 @@ + RawStringHeaderFileExtensions, HeaderFileExtensions)) { +llvm::errs() << "Invalid header file extension: " +

Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check

alexfh added a comment. There are still a few comments open. One more important thing is to try running this check over a large enough project (LLVM + Clang, for example), apply fixes, look at the results and try to build the fixed code and run all tests. You can use the clang-tidy/tool/run-cla

Re: [PATCH] D16882: [clang-tidy] More friendly warning in "google-runtime-references" when meeting an unnamed function parameter.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good. Thank you! Repository: rL LLVM http://reviews.llvm.org/D16882 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists

Re: [PATCH] D16113: [clang-tdiy] Add header file extension configuration support.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good with a couple of comments. Thanks for working on this! Comment at: clang-tidy/utils/HeaderFileExtensionsUtils.cpp:45 @@ +44,3 @@ + +bool parseHeaderFileExtensions(l

Re: [PATCH] D16113: [clang-tdiy] Add header file extension configuration support.

alexfh added inline comments. Comment at: clang-tidy/misc/DefinitionsInHeadersCheck.cpp:40 @@ +39,3 @@ +',')) { +// FIXME(hokein): Find a more suitable way to handle invalid configuration +// options. It's common to

Re: [PATCH] D16700: [Clang-tidy] Make null pointer literals for fixes configurable for two checks

alexfh added a comment. Please include full context to the diffs. See http://llvm.org/docs/Phabricator.html for instructions. Repository: rL LLVM http://reviews.llvm.org/D16700 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists

Re: [PATCH] D16113: [clang-tdiy] Add header file extension configuration support.

alexfh added inline comments. Comment at: clang-tidy/utils/HeaderFileExtensionsUtils.cpp:46 @@ +45,3 @@ +bool parseHeaderFileExtensions(llvm::StringRef AllHeaderFileExtensions, + HeaderFileExtensionsSet &HeaderFileExtensions, +

Re: [PATCH] D16113: [clang-tdiy] Add header file extension configuration support.

alexfh added a comment. Still looks good. http://reviews.llvm.org/D16113 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16113: [clang-tdiy] Add header file extension configuration support.

alexfh added a comment. A couple of post-commit comments. Comment at: clang-tools-extra/trunk/clang-tidy/google/GlobalNamesInHeadersCheck.h:24 @@ +23,3 @@ +/// +/// The check supports these options: +/// - `HeaderFileExtensions`: a comma-separated list of filename extensions

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

alexfh added a comment. In http://reviews.llvm.org/D16517#336157, @flx wrote: > In http://reviews.llvm.org/D16517#336148, @alexfh wrote: > > > A high-level comment: I think, this comment > > still applies. I'm also slightly > > concerned about having this

Re: [PATCH] D16310: new clang-tidy checker misc-long-cast

alexfh added inline comments. Comment at: clang-tidy/misc/MisplacedWideningCastCheck.cpp:20 @@ +19,3 @@ +void MisplacedWideningCastCheck::registerMatchers(MatchFinder *Finder) { + auto Calc = stmt(anyOf(binaryOperator(anyOf( + hasOperatorName("+"), has

Re: [PATCH] D16922: [clang-tidy] Added check-fixes for misc-virtual-near-miss.

alexfh added inline comments. Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:252 @@ -251,1 +251,3 @@ // A "virtual near miss" is found. +auto Range = CharSourceRange::getTokenRange(SourceRange( +DerivedMD->getLocation(), DerivedMD->ge

[clang-tools-extra] r260065 - [clang-tidy] Reformatted docs + minor updates

Author: alexfh Date: Sun Feb 7 18:19:29 2016 New Revision: 260065 URL: http://llvm.org/viewvc/llvm-project?rev=260065&view=rev Log: [clang-tidy] Reformatted docs + minor updates Modified: clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp clang-tools-extra/trunk/docs/clang-tidy/in

Re: [PATCH] D16764: [clang-tidy] Move incorrect-roundings to upstream.

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

Re: [PATCH] D16979: [clang-tidy] Some improvements in 'misc-definitions-in-headers' check.

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/D16979 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/ma

Re: [PATCH] D16926: [clang-tidy] Fix assertion failure on `at` function in modernize-loop-convert.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thank you! http://reviews.llvm.org/D16926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listi

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

alexfh added a comment. Looks good apart from renaming and one comment that's still relevant. Comment at: clang-tidy/misc/UninitializedFieldCheck.cpp:178 @@ +177,3 @@ + + SmallPtrSet FieldsToInit; + fieldsRequiringInit(MemberFields, FieldsToInit); What about t

[clang-tools-extra] r260099 - [clang-tidy] Fix an error in .rst

Author: alexfh Date: Mon Feb 8 09:09:34 2016 New Revision: 260099 URL: http://llvm.org/viewvc/llvm-project?rev=260099&view=rev Log: [clang-tidy] Fix an error in .rst Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-incorrect-roundings.rst Modified: clang-tools-extra/trunk/docs

[clang-tools-extra] r260100 - [clang-tidy] Reshuffled paragraphs a bit, added contents.

Author: alexfh Date: Mon Feb 8 09:09:39 2016 New Revision: 260100 URL: http://llvm.org/viewvc/llvm-project?rev=260100&view=rev Log: [clang-tidy] Reshuffled paragraphs a bit, added contents. Modified: clang-tools-extra/trunk/docs/clang-tidy/index.rst Modified: clang-tools-extra/trunk/docs/cl

Re: [PATCH] D16310: new clang-tidy checker misc-long-cast

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good after fixing capitalization in comments. Thank you! Comment at: clang-tidy/misc/MisplacedWideningCastCheck.cpp:32 @@ +31,3 @@ + has(C

Re: [PATCH] D16987: [clang-tidy] Correct IncorrectRoundings namespace.

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/D16987 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/ma

Re: [PATCH] D16535: [clang-tidy] Check to find unintended semicolons that changes the semantics.

alexfh added a comment. Sorry for the long delay. Currently rather swamped. Haojian, could you take a look at this patch? http://reviews.llvm.org/D16535 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/l

Re: [PATCH] D16922: [clang-tidy] Added check-fixes for misc-virtual-near-miss.

alexfh added inline comments. Comment at: test/clang-tidy/misc-virtual-near-miss.cpp:1 @@ -1,2 +1,2 @@ // RUN: %check_clang_tidy %s misc-virtual-near-miss %t The check shouldn't make any changes to template classes based on any single instantiation. An easy wa

Re: [PATCH] D16953: Enhance clang-tidy modernize-redundant-void-arg check to apply fixes to header files

alexfh added a comment. Hi Richard, Thank you for working on this. The script has been begging for refactoring for a while ;) A couple of initial comments, and while I'm looking further into into this patch, could you find other possible usages (in currently existing tests) of the new function

Re: [PATCH] D16962: clang-tidy: avoid std::bind

alexfh added a comment. In http://reviews.llvm.org/D16962#346822, @jbcoe wrote: > Moved check to readability module. > > Aside: would it be worthwhile creating a python script to move checks from > one module to another? It's reasonable to teach the `rename_check.py` script move across modules

Re: [clang-tools-extra] r260217 - [clang-tidy] Make readability-container-size-empty work with inline namespaces. Fix PR25812.

I think, we need to add a `hasAnyName()` matcher that would take a list of names. On Tue, Feb 9, 2016 at 11:20 AM, Gabor Horvath via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: xazax > Date: Tue Feb 9 04:20:48 2016 > New Revision: 260217 > > URL: http://llvm.org/viewvc/llvm-projec

Re: [PATCH] D16922: [clang-tidy] Added check-fixes for misc-virtual-near-miss.

alexfh added inline comments. Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:183 @@ +182,3 @@ +/// +/// This function is copied from the one defined in ASTMatchFinder.cpp to solve +/// the problem that QualType::getAsCXXRecordDecl does not work for template

Re: [clang-tools-extra] r260225 - [clang-tidy] Add -target in misc-misplaced-widening-cast test so it will work on various bots

I'm not sure this is a good fix. What was the specific issue (and on which buildbots)? On Tue, Feb 9, 2016 at 4:43 PM, Daniel Marjamaki via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: danielmarjamaki > Date: Tue Feb 9 09:43:05 2016 > New Revision: 260225 > > URL: http://llvm.org/v

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

alexfh added inline comments. Comment at: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:88 @@ +87,3 @@ +if (InitializerBefore != nullptr) + return InitializerBefore->getRParenLoc().getLocWithOffset(1); +auto StartLocation = InitializerAfter != nullptr --

Re: [PATCH] D17064: Only invoke ForRangeCopyCheck on expensive-to-copy types.

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/D17064 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/ma

Re: SV: [clang-tools-extra] r260225 - [clang-tidy] Add -target in misc-misplaced-widening-cast test so it will work on various bots

Makes sense. I wonder though whether the check should attempt to be platform-independent and warn regardless of whether int and long have the same bit width. What do you think? On Feb 10, 2016 9:35 AM, "Daniel Marjamäki" wrote: > > Hello! > > > I'm not sure this is a good fix. What was the specif

Re: [clang-tools-extra] r260225 - [clang-tidy] Add -target in misc-misplaced-widening-cast test so it will work on various bots

On Wed, Feb 10, 2016 at 1:15 PM, Daniel Marjamäki < daniel.marjam...@evidente.se> wrote: > > Hello! > > That would make some sense to me. > > To catch portability issues users could use different relevant targets. Do > you know if some do that? > They could, but I'm not sure it's a widespread pra

Re: [PATCH] D16922: [clang-tidy] Added check-fixes for misc-virtual-near-miss.

alexfh added a comment. > The strategy has changed. Now this check does not ignore template in > instantiation, instead, it generate a > single warning for some instantiation and ignore others, so that fix is able > to apply. I'm not sure this is the right thing to do, since there might be

Re: [PATCH] D16535: [clang-tidy] Check to find unintended semicolons that changes the semantics.

alexfh accepted this revision. alexfh added a comment. LG. Thanks! Could you run the check on LLVM and post here a summary of results: how many warnings are generated, whether there are any false positives (based on a reasonably-sized random sample, if there are too many warnings to inspect all

Re: [PATCH] D17069: [clang-tidy] Fix an assert failure in 'readability-braces-around-statements' check.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good with one possible improvement. Thank you! Comment at: clang-tidy/readability/BracesAroundStatementsCheck.cpp:181 @@ +180,3 @@ + // IfStmt. Avoid triggering assert

Re: [PATCH] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers

On Wed, Feb 10, 2016 at 7:19 AM, Richard via cfe-commits < cfe-commits@lists.llvm.org> wrote: > > [Please reply *only* to the list and do not include my email directly > in the To: or Cc: of your reply; otherwise I will not see your reply. > Thanks.] > > In article , > Richard via cfe-commits

Re: [PATCH] D17043: Check that the result of a library call w/o side effects is used

alexfh added a comment. Thank you for working on this check! I'd like to note that there is a library-side way to mitigate this issue using the `[[clang::warn_unused_result]]` attribute on `vector::empty()` and similar methods: $ cat q.cc #if defined(__clang__) # if __cplusplus >= 201103

Re: [PATCH] D17043: Check that the result of a library call w/o side effects is used

alexfh added a comment. In http://reviews.llvm.org/D17043#348569, @sidney wrote: > In http://reviews.llvm.org/D17043#348525, @alexfh wrote: > > > I'd like to note that there is a library-side way to mitigate this issue > > using the `[[clang::warn_unused_result]]` attribute on `vector::empty()`

Re: [PATCH] D17134: [clang-tidy] Fix an assert failure of ForStmt in `readability-braces-around-statements` check.

alexfh added inline comments. Comment at: test/clang-tidy/readability-braces-around-statements-assert-failure.cpp:13 @@ +12,3 @@ + std::vector e; + for (typename std::vector::const_reverse_iterator iter = e.begin(), + end2 =

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

alexfh added inline comments. Comment at: clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:206 @@ +205,3 @@ + // Do not propose fixes in macros since we cannot place them correctly. + if (Ctor->getLocStart().isMacroID()) +return; IIUC what this is doi

Re: [PATCH] D16922: [clang-tidy] Added check-fixes for misc-virtual-near-miss.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good now. Thank you! http://reviews.llvm.org/D16922 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/m

Re: [PATCH] D15797: [clang-tidy] Fix readability-braces-around-statements assert failure

alexfh added a comment. An alternative fix has recently been committed. Can you run clang-tidy built after r260505 on your code? Repository: rL LLVM http://reviews.llvm.org/D15797 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://li

Re: [PATCH] D17140: [clang-tidy] improve misc-misplaced-widening-cast so it also detects portability problems

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/D17140 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/ma

[clang-tools-extra] r260535 - [clang-tidy] google-runtime-int: fix a false positive in implicit code.

Author: alexfh Date: Thu Feb 11 10:22:58 2016 New Revision: 260535 URL: http://llvm.org/viewvc/llvm-project?rev=260535&view=rev Log: [clang-tidy] google-runtime-int: fix a false positive in implicit code. Modified: clang-tools-extra/trunk/clang-tidy/google/IntegerTypesCheck.cpp clang-tool

Re: [PATCH] D17134: [clang-tidy] Fix an assert failure of ForStmt in `readability-braces-around-statements` check.

alexfh added inline comments. Comment at: test/clang-tidy/readability-braces-around-statements-assert-failure.cpp:13 @@ +12,3 @@ + std::vector e; + for (typename std::vector::const_reverse_iterator iter = e.begin(), + end2 =

Re: [PATCH] D17163: [ASTMatchers] Add matcher hasAnyName.

alexfh added a subscriber: alexfh. Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:644 @@ -643,2 +643,3 @@ explicit HasNameMatcher(StringRef Name); + explicit HasNameMatcher(ArrayRef Names); Why not `ArrayRef`? http://reviews.llvm.org/D17163

Re: [PATCH] D17163: [ASTMatchers] Add matcher hasAnyName.

alexfh added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:644 @@ -643,2 +643,3 @@ explicit HasNameMatcher(StringRef Name); + explicit HasNameMatcher(ArrayRef Names); bkramer wrote: > alexfh wrote: > > Why not `ArrayRef`? > Tha

Re: [PATCH] D17134: [clang-tidy] Fix an assert failure of ForStmt in `readability-braces-around-statements` check.

alexfh added inline comments. Comment at: test/clang-tidy/readability-braces-around-statements-assert-failure.cpp:8 @@ +7,3 @@ + +#include + Tests shouldn't include any library headers. This is problematic in many different ways: * standard library can be di

Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check

alexfh added a comment. Sorry for the delay. I'm trying to prioritize reviews that are taking less time per-iteration. Unfortunately, here we have a bunch of disagreements and I have to spend significant time to read through your arguments and address your points. Comment at:

Re: [PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good! Thank you for the new awesome check and to Jonathan for the original patch! http://reviews.llvm.org/D16517 ___ cfe-commits mailing l

Re: [PATCH] D16953: Enhance clang-tidy modernize-redundant-void-arg check to apply fixes to header files

alexfh added a comment. Unfortunately, the review of the changes in the script might take some time. I think, we can submit the fix itself already and work on the rest of the patch after that. http://reviews.llvm.org/D16953 ___ cfe-commits mailing

Re: [PATCH] D16953: Enhance clang-tidy modernize-redundant-void-arg check to apply fixes to header files

This revision was automatically updated to reflect the committed changes. Closed by commit rL260948: [clang-tidy] Enhance modernize-redundant-void-arg check to apply fixes to… (authored by alexfh). Changed prior to commit: http://reviews.llvm.org/D16953?vs=47319&id=48054#toc Repository: rL L

[clang-tools-extra] r260948 - [clang-tidy] Enhance modernize-redundant-void-arg check to apply fixes to header files

Author: alexfh Date: Tue Feb 16 03:49:05 2016 New Revision: 260948 URL: http://llvm.org/viewvc/llvm-project?rev=260948&view=rev Log: [clang-tidy] Enhance modernize-redundant-void-arg check to apply fixes to header files Fixes http://llvm.org/PR25894 Patch by Richard Thomson! Differential revis

Re: [PATCH] D16953: Enhance clang-tidy modernize-redundant-void-arg check to apply fixes to header files

alexfh added a comment. I've committed the changes to the check code. Can you submit the change to the script as a separate patch? Repository: rL LLVM http://reviews.llvm.org/D16953 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://

Re: [PATCH] D16953: Enhance clang-tidy modernize-redundant-void-arg check to apply fixes to header files

alexfh added a comment. And thank you for the fix, btw! Repository: rL LLVM http://reviews.llvm.org/D16953 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17134: [clang-tidy] Fix an assert failure of ForStmt in `readability-braces-around-statements` check.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good. Thank you! http://reviews.llvm.org/D17134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

Re: [PATCH] D17335: [clang-tidy] Fix a crash issue when clang-tidy runs with compilation database.

alexfh added a comment. Thank you for working on this! This change needs a lit test (ensure it fails/crashes without the patch and works fine after the patch). Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:343 @@ +342,3 @@ +.get

<    5   6   7   8   9   10   11   12   13   14   >