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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
