hokein added inline comments.
================ Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:219 + // us whether the ref results is completed. + RQuest.Limit = 100; + if (auto ID = getSymbolID(RenameDecl)) ---------------- ilya-biryukov wrote: > Why do we need to limit the number of references in the first place? I was thinking this is for performance, if the index returns too many results (the number of affected files >> our limit), we will ends up doing many unnecessary work (like resolving URIs). Removed it now, it seems a premature optimization, we can revisit this afterwards. ================ Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:274 +llvm::Expected<FileEdits> +renameOutsideFile(const NamedDecl *RenameDecl, llvm::StringRef MainFilePath, + llvm::StringRef NewName, const SymbolIndex *Index, ---------------- ilya-biryukov wrote: > NIT: Maybe call it `renameWithIndex` instead? > It should capture what this function is doing better... > > But up to you I would keep the current name, as it is symmetrical to the `renameWithinFile`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69263/new/ https://reviews.llvm.org/D69263 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
