[PATCH] D93683: [clangd] Do not take stale definition from the static index.

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added a comment.

In D93683#2468842 <https://reviews.llvm.org/D93683#2468842>, @sammccall wrote:

> I'm not 100% sure this bug would block adding the equivalent change for `fuzzyFind` though - it'll affect documentSymbol but not code-completion IIUC (because of the workaround I mentioned above). And these symbols are pretty rare.

With the similar change for `fuzzyFind()`, `WorkspaceSymbols.Macros` test fails. But maybe I can create one more review for this and we will discuss it there.



================
Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:434
+      PreambleSymbols.update(
+          *FilePath, std::make_unique<SymbolSlab>(std::move(*IF->Symbols)),
+          std::make_unique<RefSlab>(),
----------------
sammccall wrote:
> ArcsinX wrote:
> > We do not need this change to fix behaviour for removed definition, but this is the only one place where we use URI instead of path as a key.
> Yeah... nevertheless the key is totally arbitrary here, so we might as well use whatever is most convenient/cheapest.
It's not arbitrary after D93393 (we use these keys to create the list of indexed files).

But as it turns, the problem is deeper... I will try to explain.
Preamble index does not contain references, so with that change our logic about "file X is in the dynamic index, so toss references with location in X from the static index" breaks here, seems we can not think about preamble index that it is a part of the dynamic index for the background index (static). I wonder why tests do not catch this, but with this change find-all-references does not contain refs from headers for which we have preamble index, that's why I reverted this. Maybe we need to add some comment here about this behavior.
Example:
`test.h`
```
void test();
```
`test.c`
```
#include "test.h"
void te^st {}
```

- didOpen `test.c`
- references (where ^ is)
With this change only 1 element will be returned (definition of `test` in `test.c` and nothing from `test.h`)

I am not sure what we can do here, but I think we need some special merge for preamble and main file indexes (another merge class for this case instead of `MergedIndex`?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93683