omtcyfz abandoned this revision.
omtcyfz added a comment.
Oops, wrong patch... Abandoning this one anyway.
https://reviews.llvm.org/D23651
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
omtcyfz updated this revision to Diff 70193.
omtcyfz added a comment.
Revert diff, as the last one "deletes and creates" files instead of "moving and
changing them" in the filesystem.
https://reviews.llvm.org/D23651
Files:
CMakeLists.txt
TemplatedClassFunction.cpp
clang-refactor/CMakeLis
vmiklos added a comment.
Ah, if you mean you squashed this into https://reviews.llvm.org/D24192, then I
see what you mean, ignore me. :-)
https://reviews.llvm.org/D23651
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/
omtcyfz added a comment.
In https://reviews.llvm.org/D23651#531884, @vmiklos wrote:
> Kirill, are you waiting for me on this one? AFAICS the patch in its current
> form applies on top of current trunk, but no longer builds.
Miklos, it is a bit more complicated :)
Bascially, I have another pat
vmiklos added a comment.
Kirill, are you waiting for me on this one? AFAICS the patch in its current
form applies on top of current trunk, but no longer builds.
https://reviews.llvm.org/D23651
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
vmiklos added a comment.
> If I understood correctly, you and Alex are talking about different things.
Yes, sorry, the context of my above comment was the cl::opt instances in the
tool itself, not the various parameters to the different actions called from
the tool.
https://reviews.llvm.org/
omtcyfz added a comment.
Actually, it might make sense to merge `rename-at` and `rename-all` after all.
Passing a list of `old-name`/`offset` -> `new-name` is just easier.
But that's for another patch.
https://reviews.llvm.org/D23651
___
cfe-commi
omtcyfz updated this revision to Diff 69214.
omtcyfz marked 2 inline comments as done.
omtcyfz added a comment.
Address couple of comments.
https://reviews.llvm.org/D23651
Files:
clang-rename/USRFindingAction.cpp
clang-rename/USRFindingAction.h
clang-rename/tool/ClangRename.cpp
Index: cl
omtcyfz marked an inline comment as done.
omtcyfz added a comment.
In https://reviews.llvm.org/D23651#521031, @vmiklos wrote:
> It is expected that either SymbolOffsets or OldNames is empty, and the size
> of the non-empty container is the same as the size of the NewNames container.
> So no, th
vmiklos added a comment.
It is expected that either SymbolOffsets or OldNames is empty, and the size of
the non-empty container is the same as the size of the NewNames container. So
no, the code does not rely on the offsets and the old names having the same
length.
https://reviews.llvm.org/D2
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-rename/USRFindingAction.cpp:145
@@ +144,3 @@
+ explicit NamedDeclFindingConsumer(
+ ArrayRef SymbolOffsets,
+ ArrayRef OldNames,
What about the s
omtcyfz marked an inline comment as done.
Comment at: clang-rename/USRFindingAction.cpp:190
@@ -175,6 +189,3 @@
+ USRList.push_back(Finder.Find());
}
}
Yep. Makes sense. Thanks!
Comment at: clang-rename/USRFindingAction.cpp:201-202
@
omtcyfz updated this revision to Diff 68668.
https://reviews.llvm.org/D23651
Files:
clang-rename/USRFindingAction.cpp
clang-rename/USRFindingAction.h
clang-rename/tool/ClangRename.cpp
Index: clang-rename/tool/ClangRename.cpp
=
omtcyfz updated this revision to Diff 68667.
omtcyfz marked an inline comment as done.
https://reviews.llvm.org/D23651
Files:
clang-rename/USRFindingAction.cpp
clang-rename/USRFindingAction.h
clang-rename/tool/ClangRename.cpp
Index: clang-rename/tool/ClangRename.cpp
===
alexshap added inline comments.
Comment at: clang-rename/USRFindingAction.cpp:190
@@ -175,1 +189,3 @@
+ std::vector NextUSRBunch = Finder.Find();
+ USRList.push_back(NextUSRBunch);
}
USRList.push_back(std::move(NextUSRBunch))
or (IMO even better)
alexshap added inline comments.
Comment at: clang-rename/USRFindingAction.cpp:205
@@ -196,2 +204,3 @@
+ USRList));
return std::move(Consumer);
}
not particularly important, probably it can be simply
return llvm::make_uniq
omtcyfz updated this revision to Diff 68665.
omtcyfz added a comment.
Address one more comment.
https://reviews.llvm.org/D23651
Files:
clang-rename/USRFindingAction.cpp
clang-rename/USRFindingAction.h
clang-rename/tool/ClangRename.cpp
Index: clang-rename/tool/ClangRename.cpp
omtcyfz marked 2 inline comments as done.
Comment at: clang-rename/USRFindingAction.cpp:69
@@ -69,3 +68,3 @@
}
-USRs->insert(USRs->end(), USRSet.begin(), USRSet.end());
+return std::vector(USRSet.begin(), USRSet.end());
}
Ah, sure. Sorry, didn't un
omtcyfz marked an inline comment as done.
Comment at: clang-rename/USRFindingAction.cpp:69
@@ -69,2 +68,3 @@
}
-USRs->insert(USRs->end(), USRSet.begin(), USRSet.end());
+USRs.insert(USRs.end(), USRSet.begin(), USRSet.end());
+return USRs;
alexfh w
omtcyfz updated this revision to Diff 68656.
omtcyfz marked 2 inline comments as done.
omtcyfz added a comment.
Partly address comments.
https://reviews.llvm.org/D23651
Files:
clang-rename/USRFindingAction.cpp
clang-rename/USRFindingAction.h
clang-rename/tool/ClangRename.cpp
Index: clang
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-rename/USRFindingAction.cpp:69
@@ -69,2 +68,3 @@
}
-USRs->insert(USRs->end(), USRSet.begin(), USRSet.end());
+USRs.insert(USRs.end(), USRSet.begin(), USRSet.end
omtcyfz updated this revision to Diff 68511.
omtcyfz added a comment.
Prevent std::vector from redundant copying.
https://reviews.llvm.org/D23651
Files:
clang-rename/USRFindingAction.cpp
clang-rename/USRFindingAction.h
clang-rename/tool/ClangRename.cpp
Index: clang-rename/tool/ClangRenam
omtcyfz marked an inline comment as done.
Comment at: clang-rename/USRFindingAction.h:38-41
@@ -37,6 +37,6 @@
private:
- unsigned SymbolOffset;
- std::string OldName;
- std::string SpellingName;
- std::vector USRs;
+ const std::vector &SymbolOffsets;
+ const std::vector &Ol
alexshap added a subscriber: alexshap.
Comment at: clang-rename/USRFindingAction.h:38
@@ -37,5 +37,3 @@
private:
- unsigned SymbolOffset;
- std::string OldName;
- std::string SpellingName;
- std::vector USRs;
+ std::vector SymbolOffsets;
+ std::vector OldNames;
vmiklos added a comment.
I can confirm that with this, the test script from the mail thread shows that
clang-rename is almost as fast as clang++ as expected. Thanks!
https://reviews.llvm.org/D23651
___
cfe-commits mailing list
cfe-commits@lists.llv
omtcyfz updated this revision to Diff 68503.
omtcyfz added a comment.
Prevent unnecessary `std::vector` copying. Explicitly write type.
https://reviews.llvm.org/D23651
Files:
clang-rename/USRFindingAction.cpp
clang-rename/USRFindingAction.h
clang-rename/tool/ClangRename.cpp
Index: clang-
26 matches
Mail list logo
