[PATCH] D89670: [clangd] Store the containing symbol for refs

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

(I will wait for a response about the containers for top-level decls before committing.)



================
Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:803
+
+  AssertSameContainer("toplevel1", "toplevel2");
+  AssertSameContainer("classscope1", "classscope2");
----------------
kadircet wrote:
> nit: i would suggest writing these in the form of:
> 
> ```
> auto Container = [](StringRef RangeName){
>   auto Ref = findRefWithRange(RangeName;
>   ASSERT_TRUE(Ref);
>   ASSERT_FALSE(Ref->Container.isNull());
>   return Ref->Container;
> };
> EXPECT_EQ(Container("sym1"), Container("sym2"));
> EXPECT_NE(Container("sym1"), Container("sym2"));
> ```
> 
> then you could update the ones above as:
> ```
> EXPECT_EQ(Container("rangename"), findSymbol(Symbols, "symname"));
> ```
Revised as suggested, except for allowing a null container for top-level decls.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89670