[PATCH] D58341: [clangd] Index UsingDecls

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 19 07:43:00 PST 2019


ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM from my side, with a few NITs.
But let's wait for an LGTM from Haojian too, to make sure his concerns are addressed.



================
Comment at: unittests/clangd/SymbolCollectorTests.cpp:1126
+TEST_F(SymbolCollectorTest, UsingDecl) {
+  auto completions = [](const Annotations &Test, SymbolSlab SS) {
+    class IgnoreDiagnostics : public DiagnosticsConsumer {
----------------
That's definitely too much setup for such a simple test.
I thought it's possible to wire up a real index in the completion tests, but it seems that's not the case. So let's not bother to run an actual completion here, ignore my previous comment about adding a test.



================
Comment at: unittests/clangd/SymbolCollectorTests.cpp:1163
+      Contains(AllOf(
+          ReturnType(/*Currently using decls does not export target info*/ ""),
+          CompletionQName("std::foo"))));
----------------
a typo NIT: s/does not/do not
also, maybe use "type info" or "signature" instead of "target info"?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58341





More information about the cfe-commits mailing list