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

Kirill Bobyrev via cfe-commits cfe-commits at lists.llvm.org
Sun Aug 21 04:18:50 PDT 2016


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, the code does not rely on the offsets and the old names having the same length.


If I understood correctly, you and Alex are talking about different things.

As far as I understand Alex what he meant is the code inside `USRFindingAction.cpp` relies on `SymbolOffsetsVector` and `OldNamesVector` (or whatever I actually called them) having same size. And that's right. See `clang-rename/USRFindingAction.cpp` line 153 in current diff. And I actually construct them to have the same size (see line 212 in `clang-rename/ClangRename.cpp`). I do not claim this to be the best solution, open to any other ideas, but it isn't too ugly to do so IMO.

If I understand you correctly code doesn't rely on `SymbolOffsets` and `OldNames` having equal size. And that's also correct.

Thus said, what Alex proposes (bind offset and old-name together while passing to `USRFindingAction`) makes sense to me.


https://reviews.llvm.org/D23651





More information about the cfe-commits mailing list