Author: Tom Praschan Date: 2022-09-15T13:02:58+02:00 New Revision: 220d850823494aad48d984395512e2ac74c666de
URL: https://github.com/llvm/llvm-project/commit/220d850823494aad48d984395512e2ac74c666de DIFF: https://github.com/llvm/llvm-project/commit/220d850823494aad48d984395512e2ac74c666de.diff LOG: [clangd] Fix hover on symbol introduced by using declaration This fixes https://github.com/clangd/clangd/issues/1284. The example there was C++20's "using enum", but I noticed that we had the same issue for other using-declarations. Differential Revision: https://reviews.llvm.org/D133664 Added: Modified: clang-tools-extra/clangd/Hover.cpp clang-tools-extra/clangd/unittests/HoverTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp index 2ef6104e3a43..b587d9c5f1e7 100644 --- a/clang-tools-extra/clangd/Hover.cpp +++ b/clang-tools-extra/clangd/Hover.cpp @@ -1006,6 +1006,37 @@ void maybeAddCalleeArgInfo(const SelectionTree::Node *N, HoverInfo &HI, HI.CallPassType.emplace(PassType); } +const NamedDecl *pickDeclToUse(llvm::ArrayRef<const NamedDecl *> Candidates) { + if (Candidates.empty()) + return nullptr; + + // This is e.g the case for + // namespace ns { void foo(); } + // void bar() { using ns::foo; f^oo(); } + // One declaration in Candidates will refer to the using declaration, + // which isn't really useful for Hover. So use the other one, + // which in this example would be the actual declaration of foo. + if (Candidates.size() <= 2) { + if (llvm::isa<BaseUsingDecl>(Candidates.front())) + return Candidates.back(); + return Candidates.front(); + } + + // For something like + // namespace ns { void foo(int); void foo(char); } + // using ns::foo; + // template <typename T> void bar() { fo^o(T{}); } + // we actually want to show the using declaration, + // it's not clear which declaration to pick otherwise. + auto BaseDecls = llvm::make_filter_range(Candidates, [](const NamedDecl *D) { + return llvm::isa<BaseUsingDecl>(D); + }); + if (std::distance(BaseDecls.begin(), BaseDecls.end()) == 1) + return *BaseDecls.begin(); + + return Candidates.front(); +} + } // namespace llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos, @@ -1081,11 +1112,11 @@ llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos, // FIXME: Fill in HighlightRange with range coming from N->ASTNode. auto Decls = explicitReferenceTargets(N->ASTNode, DeclRelation::Alias, AST.getHeuristicResolver()); - if (!Decls.empty()) { - HI = getHoverContents(Decls.front(), PP, Index, TB); + if (const auto *DeclToUse = pickDeclToUse(Decls)) { + HI = getHoverContents(DeclToUse, PP, Index, TB); // Layout info only shown when hovering on the field/class itself. - if (Decls.front() == N->ASTNode.get<Decl>()) - addLayoutInfo(*Decls.front(), *HI); + if (DeclToUse == N->ASTNode.get<Decl>()) + addLayoutInfo(*DeclToUse, *HI); // Look for a close enclosing expression to show the value of. if (!HI->Value) HI->Value = printExprValue(N, AST.getASTContext()); diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp index d4000f31a5f7..d2ad2c69c4e7 100644 --- a/clang-tools-extra/clangd/unittests/HoverTests.cpp +++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp @@ -1630,6 +1630,38 @@ TEST(Hover, All) { HI.Type = "int"; HI.Definition = "int foo"; }}, + { + R"cpp(// Function definition via using declaration + namespace ns { + void foo(); + } + int main() { + using ns::foo; + ^[[foo]](); + } + )cpp", + [](HoverInfo &HI) { + HI.Name = "foo"; + HI.Kind = index::SymbolKind::Function; + HI.NamespaceScope = "ns::"; + HI.Type = "void ()"; + HI.Definition = "void foo()"; + HI.Documentation = ""; + HI.ReturnType = "void"; + HI.Parameters = std::vector<HoverInfo::Param>{}; + }}, + { + R"cpp( // using declaration and two possible function declarations + namespace ns { void foo(int); void foo(char); } + using ns::foo; + template <typename T> void bar() { [[f^oo]](T{}); } + )cpp", + [](HoverInfo &HI) { + HI.Name = "foo"; + HI.Kind = index::SymbolKind::Using; + HI.NamespaceScope = ""; + HI.Definition = "using ns::foo"; + }}, { R"cpp(// Macro #define MACRO 0 @@ -1734,6 +1766,25 @@ TEST(Hover, All) { HI.Definition = "ONE"; HI.Value = "0"; }}, + { + R"cpp(// C++20's using enum + enum class Hello { + ONE, TWO, THREE, + }; + void foo() { + using enum Hello; + Hello hello = [[O^NE]]; + } + )cpp", + [](HoverInfo &HI) { + HI.Name = "ONE"; + HI.Kind = index::SymbolKind::EnumConstant; + HI.NamespaceScope = ""; + HI.LocalScope = "Hello::"; + HI.Type = "enum Hello"; + HI.Definition = "ONE"; + HI.Value = "0"; + }}, { R"cpp(// Enumerator in anonymous enum enum { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
