[PATCH] D51481: [clangd] Implement proximity path boosting for Dex

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 30 05:05:27 PDT 2018


sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/index/dex/DexIndex.h:45
+  void build(std::shared_ptr<std::vector<const Symbol *>> Symbols,
+             llvm::ArrayRef<std::string> URISchemes);
 
----------------
ioeric wrote:
> URI schemes are property of `Symbols`. We shouldn't need to pass specify the schemes again. Dex can collect all possible URI schemes when building the index.
> 
> I think we could generate proximity paths for index symbols without knowing about schemes (when building the index). For example, we could `canonicalize` the URI (as in `FileDistance.cpp`), for example, by converting `test:///x/y/z` to `/test:/x/y/z`.  For a query, we would first convert query proximity paths to URIs (of all possible schemes), perform the same canonicalization on URIs, and then use the canonicalized paths to compute proximity.
We had an offline discussion about URIs which might be interesting.

Fetching posting lists for all URI schemes at query time seems wasteful, @ilya-biryukov pointed out that in practice each file is going to exist in some preferred URI space, and we should only compare that one.
The only obstacle to doing that is that the preference order for URI schemes is not global, each place we need it it's accepted as configuration.

Maybe we should change that: as part of registering the URI schemes, we define a preferred order. And instead of the current operation `makeURI(File, scheme)` we should just offer APIs like `makePreferredURI(File)` and `makeFileURI(File)`.


https://reviews.llvm.org/D51481





More information about the cfe-commits mailing list