[PATCH] D97535: [clangd] Use URIs instead of paths in the index file list

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added inline comments.


================
Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:292
+    for (const auto &Sym : *Slab) {
+      if (Sym.Definition)
+        Files.insert(Sym.Definition.FileURI);
----------------
kadircet wrote:
> it feels weird to choose one or the other here, why not just insert both (after checking if definition exists, ofc).
> 
> We are likely to have a merged symbol information anyway, and the logic here will likely result in no index owning the header files, unless there are some symbols *defined* (not just declared) in it.
> 
> This will likely result in some overshooting as knowing about a symbols declaration location doesn't mean indexing that particular file. But so does the current version, as knowing about definition location might be because of a merged symbol, rather than indexing of particular file.
I will try to explain on example:

- **test.h**
```
void f();
```
- **test.c**
```
#include "test.h"
void f() { }
```
- compile_commands.json contains a command for **test.c** compilation.

Scenario:
- open **test.c**
- try to find all references for `f()`

For that scenario result for `find all references` will be incorrect if both (decl and def) files are in the file list because:
- decl location is inside **test.h**
- def location is inside **test.c**
- the file list for the main file index contains **test.h** and **test.c**
- the main file index does not contain references from **test.h**
- the background (static) index contains references from **test.c**, but results from the background index will be skipped, because **test.h** is in the main file (dynamic) index file list.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97535/new/

https://reviews.llvm.org/D97535