[clang-tools-extra] [clangd] Store documentation when indexing standard library (PR #133681)

[kadir çetinkaya via cfe-commits]

kadircet wrote:

> 1.  we no longer use SymbolOrigin::StdLib for collected symbols

thanks for catching that, I think that's OK to pass in an extra `SymbolOrigin` flag here.

> 2. we run the symbol collector with CollectMainFileSymbols = true

I think this is WAI. Firstly we shouldn't have (m)any symbols in main-file when running stdlib indexing (as we just stitch together a file that `#include <...>` all stdlib headers we know of). but even if we run into some, these symbols are all part of the preamble section. hence even-if they're main-file only, they should get the same treatment as any other preamble symbol.

This also ensures we don't get different behavior if `<vector>` is indexed through preamble-index vs stdlib-index first.

> 3. we use the indexing option SystemSymbolFilterKind::DeclarationsOnly

agreed that this sounds like an improvement.

> 4. we no longer use [this](https://searchfox.org/llvm/rev/bb21a6819b3fb9d689de776f7ee768030dfbacea/clang-tools-extra/clangd/index/IndexAction.cpp#143-148) "deeply nested" optimization

I think this was just an oversight, we should probably have that in `indexSymbols` too. This can be a separate patch though.

> If that seems preferable to you over adding a parameter to createStaticIndexingAction

Just to be clear, my concern is not about adding more knobs to `createStaticIndexingAction`, but rather it being the wrong fix. As your analysis revealed we actually had many more discrepancies between dynamic-index and stdlib-index. Amending `createStaticIndexingAction` to look more like dynamic-index will just blur the line between static and dynamic index, which sounds undesired overall.

> I'm happy to revise the patch along these lines.

That would be great, thanks a lot for taking care of this!

https://github.com/llvm/llvm-project/pull/133681