sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:79
+
+ if (Index) {
+ // Consult the index to determine whether the symbol is used outside of
----------------
pull out a function for this?
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:83
+ // FIXME: we may skip querying the index if D is function-local.
+ const NamedDecl *D =
+ clang::tooling::getNamedDeclAt(ASTCtx, SourceLocationBeg);
----------------
it's unfortunate that our "what's under the cursor" logic here has to duplicate
what's done by RenameOccurrences::initiate().
Can we have RenameOccurrences expose the NamedDecl instead?
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:83
+ // FIXME: we may skip querying the index if D is function-local.
+ const NamedDecl *D =
+ clang::tooling::getNamedDeclAt(ASTCtx, SourceLocationBeg);
----------------
sammccall wrote:
> it's unfortunate that our "what's under the cursor" logic here has to
> duplicate what's done by RenameOccurrences::initiate().
>
> Can we have RenameOccurrences expose the NamedDecl instead?
does this not work for macros?
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:85
+ clang::tooling::getNamedDeclAt(ASTCtx, SourceLocationBeg);
+ if (!D)
+ return llvm::make_error<llvm::StringError>(
----------------
We're effectively doing this test twice (see `if (!Rename)` below) and emitting
different error mesages
maybe we should just skip the rest of the check in this case?
(Exposing the ND from the RenameOccurrences would help make this duplication
more obvious)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63426/new/
https://reviews.llvm.org/D63426
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
