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