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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
