[PATCH] D53273: [clangd] Fix some references missing in dynamic index.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clangd/index/SymbolCollector.cpp:348
 
+  if (!shouldCollectSymbol(*ND, *ASTCtx, Opts))
+    return true;
----------------
sammccall wrote:
> This seems better for the main-AST case, but substantially more expensive for indexing preambles: we may not want references at all, so paying for shouldCollectSymbol for every reference seems wasteful.
> 
> what about
> ```
> bool RefMayBeInteresting = RefFilter & Roles;
> bool IsOnlyRef = !(Roles & (Declaration | Definition));
> if (IsOnlyRef && !RefMayBeInteresting)
>   return;
> if (!shouldCollectSymbol())
>   return;
> if (RefMayBeInteresting && File == getMainFileID())
>   // add ref
> if (IsOnlyRef) // don't continue if mere reference
>   return;
> ```
> 
I thought `shouldCollectSymbol` is not as expensive as before (since we have removed AST matchers stuff there), but I'm definitely +1 on the idea of avoid calling it for every reference. 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53273