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

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:770
+    EXPECT_TRUE(bool(Ref));
+    if (!AllowNull)
+      EXPECT_FALSE(Ref->Container.isNull());
----------------
nit: instead of making this part of the lambda and complicating the signature I would just check this holds for classscope1 and namespacescope1 explicitly. it should be true for other cases anyways, as we are checking equality with non-null smybolids.


================
Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:782
+
+    EXPECT_EQ(Ref1->Container, Ref2->Container);
+  };
----------------
nridge wrote:
> kadircet wrote:
> > can you also assert containers here are non-null (and below)
> It looks like the container is null for toplevel decls. Is that a problem?
I think that's OK for now, might be worth leaving a comment tho. (at `Ref::Container`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89670