[PATCH] D105083: [clangd] Ensure Ref::Container refers to an indexed symbol

[Nathan Ridge via Phabricator via cfe-commits]

nridge marked an inline comment as done.
nridge added inline comments.


================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:159
+// indexed, we walk further up because Ref::Container should always be
+// an indexed symbol.
+const Decl *getRefContainer(const Decl *Enclosing,
----------------
kadircet wrote:
> can you also add some info about why we are not using `DeclContext`s ? something like:
> ```
> Decls can nest under non-DeclContext nodes, in cases like initializer, where they might be attributed to VarDecl.
> Preserving that level of granularity is especially useful for initializers at top-level, as otherwise the only context we
> can attach these refs is TranslationUnitDecl, which we don't preserve in the index.
> FIXME: Maybe we should have some symbols for representing file-scopes, that way we can show these refs as
> being contained in the file-scope.
> ```
> 
> (Last fixme bit is optional, please add that if you also think the functionality would be more useful that way)
I've added a comment to explain why we're not using`DeclContext` here.

I'm not sure if your other comment is still relevant, if we agree that the current behaviour is the desirable one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105083