[PATCH] D72874: [clangd] Add a textual fallback for go-to-definition
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 11 09:04:42 PDT 2020
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
In D72874#1915840 <https://reviews.llvm.org/D72874#1915840>, @nridge wrote:
> I've started to update the patch to be in line with the direction discussed in the issue.
>
> @sammccall, how would you like to proceed logistically:
>
> - Do you plan to land (a possibly modified version of) D75479 <https://reviews.llvm.org/D75479>?
> - Or should I combine that patch into this one?
This patch looks good, I wouldn't bother redesigning anything, we should iterate instead.
You should go ahead, and I'll merge, and then we should work towards enabling dependent code use cases etc. SG?
================
Comment at: clang-tools-extra/clangd/FindSymbols.h:25
+/// Helper function for deriving an LSP Location from a SymbolLocation.
+llvm::Expected<Location> symbolLocationToLocation(const SymbolLocation &Loc,
+ llvm::StringRef HintPath);
----------------
nit: these names are vague and echo the type signature, maybe indexToLSPLocation?
================
Comment at: clang-tools-extra/clangd/FindSymbols.h:26
+llvm::Expected<Location> symbolLocationToLocation(const SymbolLocation &Loc,
+ llvm::StringRef HintPath);
+
----------------
nit: HintPath should be TUPath, the decision to use some other path as a TU path can only be made in the caller (needs context).
(Same is true for symbolToLocation, I'm not sure when that became public)
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:486
+ Req.Limit = 10;
+ TopN<ScoredLocatedSymbol, ScoredSymbolGreater> Top(*Req.Limit);
+ FuzzyMatcher Filter(Req.Query);
----------------
(The fuzzy matcher and topN are still here - I think we don't need them, right? With only up-to-3 results, std::sort seems more obvious)
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:488
+ FuzzyMatcher Filter(Req.Query);
+ Index->fuzzyFind(Req, [&](const Symbol &Sym) {
+ auto MaybeDeclLoc =
----------------
maybe bail out early (on unusable/too many) instead of doing all the score computations first?
fuzzyFind(..., {
// bail out if it's a constructor or name doesn't match
if (Results.size() >= 3) {
TooMany = true;
return;
}
// add result
});
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72874/new/
https://reviews.llvm.org/D72874
More information about the cfe-commits
mailing list