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
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
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:
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
___
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:
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
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:
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
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
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()
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
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
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
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
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
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
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
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",
-
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",
-
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",
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:
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
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
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
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
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
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
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
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
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.
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/
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
Prazek created this revision.
Prazek added reviewers: sbenza, alexfh, aaron.ballman.
Prazek added subscribers: cfe-commits, hokein, sbarzowski, mnbvmar, staronj,
sbenza.
Prazek set the repository for this revision to rL LLVM.
Prazek added a project: clang-extra.
Not everything is valid (as you se
33 matches
Mail list logo
