HighCommander4 wrote: > I am afraid this doesn't address > [clangd/clangd#1104](https://github.com/clangd/clangd/issues/1104) to a > useful extent :/. > > Whenever a header `foo.h` changes, clangd will schedule indexing of only a > **single** translation unit that depends on it, not all of them. So if there > are other TUs that reference `foo`, they'll still stay stale.
Good catch, thanks; I had overlooked that. (I added an additional testcase demonstrating this, mostly for my own future reference / as a reference for anyone else who might pick this up.) > I believe the performance trade-offs here are overwhelming and it'll result > in pretty much re-indexing of whole project at every rebase or slight > modifications to headers (we can't satisfy correctness without rebuilding all > the TUs that depend on a changed header, rather than a single one). FWIW, my expectation as a user is that when a header is modified, that **does** trigger re-indexing of all source files that include the header. I don't think the performance impact of this is that catastrophic, as the number of headers in a project tends to vary inversely with how widely they're included (i.e. most headers will have a few includers, and a few will have many). In my mind, the scenario is comparable to modifying a header and then **rebuilding**: if the header is widely included, you will rebuild many TUs, and programmers expect that (and adopt practices like forward declarations, pimpl, etc. to avoid unnecessary header dependencies). Anyways, I agree that the patch in its current form is not useful. I'll mark it as a draft, and (when time permits) will re-work it to implement a "proper" (all dependent TUs rebuilt) update procedure. We can then discuss what the trigger for that procedure should be (explicit invocation, clangd restart, file-save of header, etc.). https://github.com/llvm/llvm-project/pull/140651 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
