[PATCH] D63426: [clangd] Narrow rename to local symbols.

[Haojian Wu via Phabricator via cfe-commits]

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