[PATCH] D63426: [clangd] Narrow rename to local symbols.
Haojian Wu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 18 08:00:50 PDT 2019
hokein added inline comments.
================
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:
> 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?
unfortunately, the rename library doesn't rename marcos.
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:85
+ clang::tooling::getNamedDeclAt(ASTCtx, SourceLocationBeg);
+ if (!D)
+ return llvm::make_error<llvm::StringError>(
----------------
sammccall wrote:
> 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)
The duplication doesn't exist if we expose the ND from the RenameOccurrence.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63426/new/
https://reviews.llvm.org/D63426
More information about the cfe-commits
mailing list