[PATCH] D23651: [clang-rename] improve performance for rename-all

[Kirill Bobyrev via cfe-commits]

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 wrote:
> Should USRs be a local variable now?
Can you elaborate please?

================
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;
----------------
alexfh wrote:
> 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?
Not really, they aren't meant to be quite huge. At this point I have ensured the lifetimes, but if the code would be reused, I agree, it might cause some trouble if one is not careful enough.

Do you propose to perform copying to gain more safety?


https://reviews.llvm.org/D23651