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());
+ return USRs;
----------------
Should USRs be a local variable now?
================
Comment at: clang-rename/USRFindingAction.cpp:147
@@ +146,3 @@
+ explicit NamedDeclFindingConsumer(
+ const std::vector<unsigned> &SymbolOffsets,
+ const std::vector<std::string> &OldNames,
----------------
Use `ArrayRef` here as well. BTW, if the code relies on `SymbolOffsets` and
`OldNames` being of the same length, maybe a single collection of pairs would
work better? Or define a structure for keeping offset and old name together?
================
Comment at: clang-rename/USRFindingAction.h:29
@@ -27,3 +28,3 @@
struct USRFindingAction {
- USRFindingAction(unsigned Offset, const std::string &Name)
- : SymbolOffset(Offset), OldName(Name) {}
+ USRFindingAction(const std::vector<unsigned> &SymbolOffsets,
+ const std::vector<std::string> &OldNames)
----------------
Use `ArrayRef` instead of `const vector<>&`. `ArrayRef<>` is less restrictive.
================
Comment at: clang-rename/USRFindingAction.h:30
@@ +29,3 @@
+ USRFindingAction(const std::vector<unsigned> &SymbolOffsets,
+ const std::vector<std::string> &OldNames)
+ : SymbolOffsets(SymbolOffsets), OldNames(OldNames) {}
----------------
Use `ArrayRef<std::string>`.
================
Comment at: clang-rename/USRFindingAction.h:38
@@ -37,5 +37,3 @@
private:
- unsigned SymbolOffset;
- std::string OldName;
- std::string SpellingName;
- std::vector<std::string> USRs;
+ const std::vector<unsigned> &SymbolOffsets;
+ const std::vector<std::string> &OldNames;
----------------
omtcyfz wrote:
> Aw, you're right. Good catch, thanks!
Reference members always seem suspicious to me. One has to be really really
careful not to mess up lifetimes. Are we actually saving much but not copying
these vectors?
https://reviews.llvm.org/D23651
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
