[PATCH] D72746: [clangd] Add a flag for implicit references in the Index

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.


================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:599
+    const auto Tokens = FilesToTokensCache[MainFileID];
+    for (auto &DeclAndRef : DeclRefs) {
+      if (auto ID = getSymbolID(DeclAndRef.first)) {
----------------
hokein wrote:
> note that the `DeclRefs` may contains references from non-main files, e.g. included headers if `RefsInHeaders` is true. I think we need to tokenize other files if the reference is not from main file.   `CollectRef` lambda is a better place to place the `FilesToTokensCache` logic.
> note that the DeclRefs may contains references from non-main files, e.g. included headers if RefsInHeaders is true. I think we need to tokenize other files if the reference is not from main file.

Fair enough, fixed that!

> CollectRef lambda is a better place to place the FilesToTokensCache logic.

I don't really agree with that. In my opinion `CollectRef` should remain a simple helper that simply puts pre-processed reference into the storage. Complicating it (and also making it work with both `MacroRefs` and `DeclRefs` while the token spelling check is only required for declarations looks suboptimal to me. If you really want me to do that, please let me know, but I personally think it might be better this way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72746