[PATCH] D71598: [clangd] Filter implicit references from index while renaming
Kadir Cetinkaya via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 18 08:53:44 PST 2019
kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/SourceCode.cpp:217
+SourceLocation positionToSourceLoc(const SourceManager &SM, Position Pos,
+ StringRef Filename) {
----------------
this one isn't used anywhere?
================
Comment at: clang-tools-extra/clangd/SourceCode.cpp:1136
+std::vector<Range> filterRenameRanges(const SourceManager &SM,
+ StringRef Filename,
----------------
SM doesn't seem to be necessary, as `lex` already provides that in the callback.
================
Comment at: clang-tools-extra/clangd/SourceCode.h:301
+/// Removes ranges with implicit references to the renamed symbol (e.g. in macro
+/// expansions).
----------------
i don't think it is necessary for this function to be made public, it should be OK for it to leave in rename.cpp as a helper.
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:368
+ // Filter out possible implicit references returned from the index.
+ const auto FilteredRanges = filterRenameRanges(
+ SM, FileAndOccurrences.first(),
----------------
this one should go after `adjustRenameRanges`
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:368
+ // Filter out possible implicit references returned from the index.
+ const auto FilteredRanges = filterRenameRanges(
+ SM, FileAndOccurrences.first(),
----------------
kadircet wrote:
> this one should go after `adjustRenameRanges`
both this and `adjustRenameRanges` seems to be lexing the source code to get identifier locations.
can we lex the file only a single time instead and make use of the result in both of the functions?
I would suggest moving `collectIdentifierRanges` into here and passing the result as a parameter to both of the functions.
as for implementation of `filterRenameRanges` you might wanna return intersection of `RenameRanges` and result of `collectIdentifierRanges`
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71598/new/
https://reviews.llvm.org/D71598
More information about the cfe-commits
mailing list