Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-08-02 Thread Piotr Padlewski via cfe-commits
Prazek marked 5 inline comments as done. Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:115 @@ -95,1 +114,3 @@ + auto CtorCallSourceRange = CharSourceRange::getTokenRange( + InnerCtorCall->getExprLoc(), CallParensRange.getBegin()); alexfh wrote: > Pr

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-08-02 Thread Piotr Padlewski via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D22208#503194, @alexfh wrote: > Please add revision number (this can be automated, if include differential > revision URL in your commit message as described in > http://llvm.org/docs/Phabricator.html#committing-a-change). I normally use arc

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-08-02 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:115 @@ -95,1 +114,3 @@ + auto CtorCallSourceRange = CharSourceRange::getTokenRange( + InnerCtorCall->getExprLoc(), CallParensRange.getBegin()); Prazek wrote: > alexfh wrote:

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-08-02 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. Please add revision number (this can be automated, if include differential revision URL in your commit message as described in http://llvm.org/docs/Phabricator.html#committing-a-change). https://reviews.llvm.org/D22208 ___

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-08-01 Thread Piotr Padlewski via cfe-commits
Prazek added inline comments. Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:115 @@ -95,1 +114,3 @@ + auto CtorCallSourceRange = CharSourceRange::getTokenRange( + InnerCtorCall->getExprLoc(), CallParensRange.getBegin()); alexfh wrote: > Prazek wrote:

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-08-01 Thread Piotr Padlewski via cfe-commits
Prazek added inline comments. Comment at: clang-tidy/modernize/UseEmplaceCheck.h:36-37 @@ -32,1 +35,4 @@ +private: + std::vector ContainersWithPushBack; + std::vector SmartPointers; }; aaron.ballman wrote: > What about `llvm::make_range()`? llvm::make_range do

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-08-01 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:115 @@ -95,1 +114,3 @@ + auto CtorCallSourceRange = CharSourceRange::getTokenRange( + InnerCtorCall->getExprLoc(), CallParensRange.getBegin()); Prazek wrote: > alexfh wrote:

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-07-28 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tidy/modernize/UseEmplaceCheck.h:36-37 @@ -32,1 +35,4 @@ +private: + std::vector ContainersWithPushBack; + std::vector SmartPointers; }; What about `llvm::make_range()`? https://reviews.llvm.org/D22208

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-07-26 Thread Piotr Padlewski via cfe-commits
Prazek added inline comments. Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:115 @@ -95,1 +114,3 @@ + auto CtorCallSourceRange = CharSourceRange::getTokenRange( + InnerCtorCall->getExprLoc(), CallParensRange.getBegin()); alexfh wrote: > This doesn't

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-07-25 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:115 @@ -95,1 +114,3 @@ + auto CtorCallSourceRange = CharSourceRange::getTokenRange( + InnerCtorCall->getExprLoc()

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-07-23 Thread Piotr Padlewski via cfe-commits
Prazek removed rL LLVM as the repository for this revision. Prazek updated this revision to Diff 65243. Prazek added a comment. Ok works right now. I don't know why but I could not reproduce the error in test file, but I manged to fix it. https://reviews.llvm.org/D22208 Files: clang-tidy/mod

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-07-23 Thread Alexander Kornienko via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D22208#492385, @Prazek wrote: > There is one bug left: > > In the ClangIncludeFixer.cpp:169 there is push back that looks like this > > Symbols.push_back(find_all_symbols::SymbolInfo( > > Split.first.trim(), > find_all_symbols::SymbolInfo::S

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-07-21 Thread Piotr Padlewski via cfe-commits
Prazek marked 6 inline comments as done. Prazek added a comment. Repository: rL LLVM https://reviews.llvm.org/D22208 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-07-21 Thread Piotr Padlewski via cfe-commits
Prazek removed rL LLVM as the repository for this revision. Prazek updated this revision to Diff 65023. Prazek marked 4 inline comments as done. https://reviews.llvm.org/D22208 Files: clang-tidy/modernize/UseEmplaceCheck.cpp clang-tidy/modernize/UseEmplaceCheck.h clang-tidy/utils/Matchers.h

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-07-21 Thread Piotr Padlewski via cfe-commits
Prazek added a subscriber: klimek. Prazek added a comment. Can you look at my previous comment? I nedd an expert for AST. Repository: rL LLVM https://reviews.llvm.org/D22208 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-07-21 Thread Piotr Padlewski via cfe-commits
Prazek added a comment. There is one bug left: In the ClangIncludeFixer.cpp:169 there is push back that looks like this Symbols.push_back(find_all_symbols::SymbolInfo( Split.first.trim(), find_all_symbols::SymbolInfo::SymbolKind::Unknown, CommaSplits[I].trim(), 1, {}, /*NumOccurrences=*/E

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-07-21 Thread Piotr Padlewski via cfe-commits
Prazek added inline comments. Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:112 @@ -95,1 +111,3 @@ + auto CtorCallSourceRange = CharSourceRange::getTokenRange( + InnerCtorCall->getExprLoc(), CallParensRange.getBegin()); alexfh wrote: > > There is a

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-07-21 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:45 @@ -33,3 +44,3 @@ hasDeclaration(functionDecl(hasName("push_back"))), - on(hasType(cxxRecordDecl(hasAnyName("std::vector", "llvm::SmallVector", -

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-07-21 Thread Piotr Padlewski via cfe-commits
Prazek added inline comments. Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:45 @@ -33,3 +44,3 @@ hasDeclaration(functionDecl(hasName("push_back"))), - on(hasType(cxxRecordDecl(hasAnyName("std::vector", "llvm::SmallVector", -

Re: [PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

2016-07-21 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:45 @@ -33,3 +44,3 @@ hasDeclaration(functionDecl(hasName("push_back"))), - on(hasType(cxxRecordDecl(hasAnyName("std::vector",

Re: [PATCH] D22208: clang-tidy] Fixes to modernize-use-emplace

2016-07-21 Thread Piotr Padlewski via cfe-commits
Prazek added inline comments. Comment at: clang-tidy/modernize/UseEmplaceCheck.h:36-37 @@ -32,1 +35,4 @@ +private: + std::vector ContainersWithPushBack; + std::vector SmartPointers; }; aaron.ballman wrote: > Why not use a SmallVector for these instead of a std:

Re: [PATCH] D22208: clang-tidy] Fixes to modernize-use-emplace

2016-07-21 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tidy/modernize/UseEmplaceCheck.h:36-37 @@ -32,1 +35,4 @@ +private: + std::vector ContainersWithPushBack; + std::vector SmartPointers; }; Why not use a SmallVector for these instead of a std::vector? Then yo

Re: [PATCH] D22208: clang-tidy] Fixes to modernize-use-emplace

2016-07-19 Thread Piotr Padlewski via cfe-commits
Prazek updated this revision to Diff 64640. Prazek marked an inline comment as done. Repository: rL LLVM https://reviews.llvm.org/D22208 Files: clang-tidy/modernize/UseEmplaceCheck.cpp clang-tidy/modernize/UseEmplaceCheck.h clang-tidy/utils/Matchers.h docs/clang-tidy/checks/modernize-u

Re: [PATCH] D22208: clang-tidy] Fixes to modernize-use-emplace

2016-07-19 Thread Piotr Padlewski via cfe-commits
Prazek marked 5 inline comments as done. Prazek added a comment. Aaron, Alex thanks for the review. After running it on llvm I also have found the private constructor bug so I also fixed it. Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:21 @@ +20,3 @@ +llvm::Optional> +g

Re: [PATCH] D22208: clang-tidy] Fixes to modernize-use-emplace

2016-07-19 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:31 @@ +30,3 @@ + +const std::string DefaultContainersWithPushBack = +"::std::vector; ::std::list; ::std::deque"; A st

Re: [PATCH] D22208: clang-tidy] Fixes to modernize-use-emplace

2016-07-19 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:21 @@ +20,3 @@ +llvm::Optional> +getHasAnyName(const std::vector &Names) { + llvm::Optional> HasNameMatcher; Looking at `VariadicFunction`, it appears that it already works

Re: [PATCH] D22208: clang-tidy] Fixes to modernize-use-emplace

2016-07-18 Thread Piotr Padlewski via cfe-commits
Prazek added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D22208 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22208: clang-tidy] Fixes to modernize-use-emplace

2016-07-16 Thread Piotr Padlewski via cfe-commits
Prazek removed rL LLVM as the repository for this revision. Prazek updated this revision to Diff 64238. Prazek marked 6 inline comments as done. https://reviews.llvm.org/D22208 Files: clang-tidy/modernize/UseEmplaceCheck.cpp clang-tidy/modernize/UseEmplaceCheck.h clang-tidy/utils/Matchers.h

Re: [PATCH] D22208: clang-tidy] Fixes to modernize-use-emplace

2016-07-12 Thread Piotr Padlewski via cfe-commits
Prazek added inline comments. Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:129 @@ -95,1 +128,3 @@ + auto CtorCallSourceRange = CharSourceRange::getTokenRange( + InnerCtorCall->getExprLoc(), CallParensRange.getBegin()); There is a bug here that I fo

Re: [PATCH] D22208: clang-tidy] Fixes to modernize-use-emplace

2016-07-11 Thread Piotr Padlewski via cfe-commits
Prazek added inline comments. Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:21 @@ +20,3 @@ +llvm::Optional> +getHasAnyName(const std::vector &names) { + llvm::Optional> hasMatcher; aaron.ballman wrote: > aaron.ballman wrote: > > Should be `Names` instead.

Re: [PATCH] D22208: clang-tidy] Fixes to modernize-use-emplace

2016-07-11 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:21 @@ +20,3 @@ +llvm::Optional> +getHasAnyName(const std::vector &names) { + llvm::Optional> hasMatcher; Should be `Names` instead. Comment at: clang-tidy/

Re: [PATCH] D22208: clang-tidy] Fixes to modernize-use-emplace

2016-07-10 Thread Piotr Padlewski via cfe-commits
Prazek added a comment. BTW I've made clang-extra project on phabricator as you can see. Please use it, it will be much easier to filter reviews. Repository: rL LLVM http://reviews.llvm.org/D22208 ___ cfe-commits mailing list cfe-commits@lists.l