[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/Hover.cpp:1023
+  // which in this example would be the actual declaration of foo.
+  if (Candidates.size() == 2) {
+    if (llvm::isa<BaseUsingDecl>(Candidates.front()))
----------------
nit: you can change this to `Candidates.size() <= 2` and get rid of the extra single candidate case above (after changing it to always return Candidate.front() when Candidate.back() is a using decl.)


================
Comment at: clang-tools-extra/clangd/Hover.cpp:1026
+      return Candidates.back();
+    if (llvm::isa<BaseUsingDecl>(Candidates.back()))
+      return Candidates.front();
----------------
just drop this if check and always return front, when it isn't a using decl.


================
Comment at: clang-tools-extra/clangd/Hover.cpp:1031
+  // For something like
+  //     namespace ns { void foo(int); void foo(char) }
+  //     using ns::fo
----------------
can we have a test case for this?


================
Comment at: clang-tools-extra/clangd/Hover.cpp:1032
+  //     namespace ns { void foo(int); void foo(char) }
+  //     using ns::fo
+  //     template <typename T> void bar() { fo^o(T{}); }
----------------
nit: `ns::foo` not `fo`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133664