[PATCH] D86047: [clangd] Target member of dependent base made visible via a using-decl

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 17 06:43:15 PDT 2020


hokein added a comment.

looks mostly good, just a few suggestions on the test.



================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:128
+const auto ValueFilter = [](const NamedDecl *D) {
+  return dyn_cast<ValueDecl>(D) != nullptr;
+};
----------------
nit: use `isa`.


================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:354
+    } else if (const UnresolvedUsingValueDecl *UUVD =
+                   dyn_cast<UnresolvedUsingValueDecl>(D)) {
+      for (const NamedDecl *Target : getMembersReferencedViaDependentName(
----------------
can we have a test for `FindTarget`? `TEST_F(TargetDeclTest, UsingDecl)` is an ideal place.


================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:738
+      )objc",
+      R"cpp(// Member of dependent base
+        template <typename T>
----------------
nit: move to `Test(LocateSymbol, Alias)`. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86047



More information about the cfe-commits mailing list