hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land.
The patch description is a bit stale now. ================ Comment at: clang-tools-extra/clangd/XRefs.cpp:1130 + std::vector<LocatedSymbol> Results; + // We rely on index to find the implementations in subclasses. + if (!Index) ---------------- The current way is probably ok -- the index may be stale (if the index is not finished yet by the time we call find-implementation), so we might loose some latest results for the current main file. I think we need some comments describing the behavior. Maybe in future, we'll reconsider it ================ Comment at: clang-tools-extra/clangd/XRefs.cpp:1140 + } + auto CurLoc = sourceLocationInMainFile(SM, Pos); + ---------------- nit: we need to verify `CurLoc` is available. ================ Comment at: clang-tools-extra/clangd/XRefs.cpp:1160 + auto DefLoc = indexToLSPLocation(Object.Definition, *MainFilePath); + Loc.Definition = DefLoc ? *DefLoc : *DeclLoc; + Results.push_back(Loc); ---------------- I think we can leave the Definition if `!DefLoc`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91626/new/ https://reviews.llvm.org/D91626 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
