kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land.
the savings look great, thanks! ================ Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:217 + llvm::SmallString<256> AbsPath = Path; + // We don't perform is_absolute check in an else branch because + // makeAbsolute might return a relative path on some InMemoryFileSystems. ---------------- i don't think the comments are applicable anymore, this was talking about the makeAbsolute inside getCanonicalPath, which is no longer the only call path. ================ Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:322 SymbolCollector::SymbolCollector(Options Opts) : Opts(std::move(Opts)) {} +SymbolCollector::~SymbolCollector() = default; ---------------- why do we need this now? ================ Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:719 + } + // Otherwise find the approprate include header for the definiting file. + if (IncludeHeader.empty()) ---------------- s/definiting/defining ================ Comment at: clang-tools-extra/clangd/index/SymbolCollector.h:176 llvm::DenseMap<const Decl *, const Decl *> CanonicalDecls; - // Cache whether to index a file or not. llvm::DenseMap<FileID, bool> FilesToIndexCache; ---------------- is this intentional ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98371/new/ https://reviews.llvm.org/D98371 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits