[clang-tools-extra] 655baae - [clangd] Show used symbols on #include line hover.
Viktoriia Bakalova via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 28 07:06:17 PDT 2023
Author: Viktoriia Bakalova
Date: 2023-03-28T14:05:34Z
New Revision: 655baae2af0b805a1c3e6d6338a32de05e342357
URL: https://github.com/llvm/llvm-project/commit/655baae2af0b805a1c3e6d6338a32de05e342357
DIFF: https://github.com/llvm/llvm-project/commit/655baae2af0b805a1c3e6d6338a32de05e342357.diff
LOG: [clangd] Show used symbols on #include line hover.
Differential Revision: https://reviews.llvm.org/D146244
Added:
Modified:
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/Hover.h
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/IncludeCleaner.h
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 27663991a7f3c..b399a12cd90cf 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -12,6 +12,7 @@
#include "CodeCompletionStrings.h"
#include "Config.h"
#include "FindTarget.h"
+#include "Headers.h"
#include "IncludeCleaner.h"
#include "ParsedAST.h"
#include "Selection.h"
@@ -38,11 +39,15 @@
#include "clang/AST/RecordLayout.h"
#include "clang/AST/Type.h"
#include "clang/Basic/CharInfo.h"
+#include "clang/Basic/LLVM.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
#include "clang/Basic/Specifiers.h"
#include "clang/Basic/TokenKinds.h"
#include "clang/Index/IndexSymbol.h"
#include "clang/Tooling/Syntax/Tokens.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringExtras.h"
@@ -52,6 +57,7 @@
#include "llvm/Support/Format.h"
#include "llvm/Support/ScopedPrinter.h"
#include "llvm/Support/raw_ostream.h"
+#include <algorithm>
#include <optional>
#include <string>
#include <vector>
@@ -1134,11 +1140,66 @@ void maybeAddSymbolProviders(ParsedAST &AST, HoverInfo &HI,
HI.Provider = spellHeader(AST, SM.getFileEntryForID(SM.getMainFileID()), H);
}
+// FIXME: similar functions are present in FindHeaders.cpp (symbolName)
+// and IncludeCleaner.cpp (getSymbolName). Introduce a name() method into
+// include_cleaner::Symbol instead.
+std::string getSymbolName(include_cleaner::Symbol Sym) {
+ std::string Name;
+ switch (Sym.kind()) {
+ case include_cleaner::Symbol::Declaration:
+ if (const auto *ND = llvm::dyn_cast<NamedDecl>(&Sym.declaration()))
+ Name = ND->getDeclName().getAsString();
+ break;
+ case include_cleaner::Symbol::Macro:
+ Name = Sym.macro().Name->getName();
+ break;
+ }
+ return Name;
+}
+
+void maybeAddUsedSymbols(ParsedAST &AST, HoverInfo &HI, const Inclusion &Inc) {
+ const SourceManager &SM = AST.getSourceManager();
+ const auto &ConvertedMainFileIncludes =
+ convertIncludes(SM, AST.getIncludeStructure().MainFileIncludes);
+ const auto &HoveredInclude = convertIncludes(SM, llvm::ArrayRef{Inc});
+ llvm::DenseSet<include_cleaner::Symbol> UsedSymbols;
+ include_cleaner::walkUsed(
+ AST.getLocalTopLevelDecls(), collectMacroReferences(AST),
+ AST.getPragmaIncludes(), SM,
+ [&](const include_cleaner::SymbolReference &Ref,
+ llvm::ArrayRef<include_cleaner::Header> Providers) {
+ if (Ref.RT != include_cleaner::RefType::Explicit ||
+ UsedSymbols.find(Ref.Target) != UsedSymbols.end())
+ return;
+
+ for (const include_cleaner::Header &H : Providers) {
+ auto MatchingIncludes = ConvertedMainFileIncludes.match(H);
+ // No match for this provider in the main file.
+ if (MatchingIncludes.empty())
+ continue;
+
+ // Check if the hovered include matches this provider.
+ if (!HoveredInclude.match(H).empty())
+ UsedSymbols.insert(Ref.Target);
+
+ // Don't look for rest of the providers once we've found a match
+ // in the main file.
+ break;
+ }
+ });
+
+ for (const auto &UsedSymbolDecl : UsedSymbols)
+ HI.UsedSymbolNames.push_back(getSymbolName(UsedSymbolDecl));
+ llvm::sort(HI.UsedSymbolNames);
+}
+
} // namespace
std::optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
const format::FormatStyle &Style,
const SymbolIndex *Index) {
+ static constexpr trace::Metric HoverCountMetric(
+ "hover", trace::Metric::Counter, "case");
PrintingPolicy PP =
getPrintingPolicy(AST.getASTContext().getPrintingPolicy());
const SourceManager &SM = AST.getSourceManager();
@@ -1157,12 +1218,14 @@ std::optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
for (const auto &Inc : AST.getIncludeStructure().MainFileIncludes) {
if (Inc.Resolved.empty() || Inc.HashLine != Pos.line)
continue;
+ HoverCountMetric.record(1, "include");
HoverInfo HI;
HI.Name = std::string(llvm::sys::path::filename(Inc.Resolved));
// FIXME: We don't have a fitting value for Kind.
HI.Definition =
URIForFile::canonicalize(Inc.Resolved, AST.tuPath()).file().str();
HI.DefinitionLanguage = "";
+ maybeAddUsedSymbols(AST, HI, Inc);
return HI;
}
@@ -1180,6 +1243,7 @@ std::optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
// Prefer the identifier token as a fallback highlighting range.
HighlightRange = Tok.range(SM).toCharRange(SM);
if (auto M = locateMacroAt(Tok, AST.getPreprocessor())) {
+ HoverCountMetric.record(1, "macro");
HI = getHoverContents(*M, Tok, AST);
if (auto DefLoc = M->Info->getDefinitionLoc(); DefLoc.isValid()) {
include_cleaner::Macro IncludeCleanerMacro{
@@ -1190,6 +1254,7 @@ std::optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
break;
}
} else if (Tok.kind() == tok::kw_auto || Tok.kind() == tok::kw_decltype) {
+ HoverCountMetric.record(1, "keyword");
if (auto Deduced = getDeducedType(AST.getASTContext(), Tok.location())) {
HI = getDeducedTypeHoverContents(*Deduced, Tok, AST.getASTContext(), PP,
Index);
@@ -1216,6 +1281,7 @@ std::optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
auto Decls = explicitReferenceTargets(N->ASTNode, DeclRelation::Alias,
AST.getHeuristicResolver());
if (const auto *DeclToUse = pickDeclToUse(Decls)) {
+ HoverCountMetric.record(1, "decl");
HI = getHoverContents(DeclToUse, PP, Index, TB);
// Layout info only shown when hovering on the field/class itself.
if (DeclToUse == N->ASTNode.get<Decl>())
@@ -1226,8 +1292,10 @@ std::optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
maybeAddCalleeArgInfo(N, *HI, PP);
maybeAddSymbolProviders(AST, *HI, include_cleaner::Symbol{*DeclToUse});
} else if (const Expr *E = N->ASTNode.get<Expr>()) {
+ HoverCountMetric.record(1, "expr");
HI = getHoverContents(N, E, AST, PP, Index);
} else if (const Attr *A = N->ASTNode.get<Attr>()) {
+ HoverCountMetric.record(1, "attribute");
HI = getHoverContents(A, AST);
}
// FIXME: support hovers for other nodes?
@@ -1378,6 +1446,29 @@ markup::Document HoverInfo::present() const {
Output.addCodeBlock(Buffer, DefinitionLanguage);
}
+ if (!UsedSymbolNames.empty()) {
+ Output.addRuler();
+ markup::Paragraph &P = Output.addParagraph();
+ P.appendText("provides ");
+
+ const unsigned long SymbolNamesLimit = 5;
+ auto Front =
+ llvm::ArrayRef(UsedSymbolNames)
+ .take_front(std::min(UsedSymbolNames.size(), SymbolNamesLimit));
+
+ for (const auto &Sym : Front) {
+ P.appendCode(Sym);
+ if (Sym != Front.back())
+ P.appendText(", ");
+ }
+
+ if (UsedSymbolNames.size() > Front.size()) {
+ P.appendText(" and ");
+ P.appendText(std::to_string(UsedSymbolNames.size() - Front.size()));
+ P.appendText(" more");
+ }
+ }
+
return Output;
}
diff --git a/clang-tools-extra/clangd/Hover.h b/clang-tools-extra/clangd/Hover.h
index 7ade177f89cc1..891a9d9ec5546 100644
--- a/clang-tools-extra/clangd/Hover.h
+++ b/clang-tools-extra/clangd/Hover.h
@@ -15,6 +15,7 @@
#include "clang/Index/IndexSymbol.h"
#include <optional>
#include <string>
+#include <vector>
namespace clang {
namespace clangd {
@@ -110,6 +111,10 @@ struct HoverInfo {
};
// Set only if CalleeArgInfo is set.
std::optional<PassType> CallPassType;
+ // Filled when hovering over the #include line. Contains the names of symbols
+ // from a #include'd file that are used in the main file, sorted in
+ // alphabetical order.
+ std::vector<std::string> UsedSymbolNames;
/// Produce a user-readable information.
markup::Document present() const;
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 8a433b0fe88d7..fb245593b2f94 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -135,27 +135,6 @@ static bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
return true;
}
-std::vector<include_cleaner::SymbolReference>
-collectMacroReferences(ParsedAST &AST) {
- const auto &SM = AST.getSourceManager();
- // FIXME: !!this is a hacky way to collect macro references.
- std::vector<include_cleaner::SymbolReference> Macros;
- auto &PP = AST.getPreprocessor();
- for (const syntax::Token &Tok :
- AST.getTokens().spelledTokens(SM.getMainFileID())) {
- auto Macro = locateMacroAt(Tok, PP);
- if (!Macro)
- continue;
- if (auto DefLoc = Macro->Info->getDefinitionLoc(); DefLoc.isValid())
- Macros.push_back(
- {Tok.location(),
- include_cleaner::Macro{/*Name=*/PP.getIdentifierInfo(Tok.text(SM)),
- DefLoc},
- include_cleaner::RefType::Explicit});
- }
- return Macros;
-}
-
llvm::StringRef getResolvedPath(const include_cleaner::Header &SymProvider) {
switch (SymProvider.kind()) {
case include_cleaner::Header::Physical:
@@ -287,6 +266,28 @@ std::vector<Diag> generateUnusedIncludeDiagnostics(
}
} // namespace
+
+std::vector<include_cleaner::SymbolReference>
+collectMacroReferences(ParsedAST &AST) {
+ const auto &SM = AST.getSourceManager();
+ // FIXME: !!this is a hacky way to collect macro references.
+ std::vector<include_cleaner::SymbolReference> Macros;
+ auto &PP = AST.getPreprocessor();
+ for (const syntax::Token &Tok :
+ AST.getTokens().spelledTokens(SM.getMainFileID())) {
+ auto Macro = locateMacroAt(Tok, PP);
+ if (!Macro)
+ continue;
+ if (auto DefLoc = Macro->Info->getDefinitionLoc(); DefLoc.isValid())
+ Macros.push_back(
+ {Tok.location(),
+ include_cleaner::Macro{/*Name=*/PP.getIdentifierInfo(Tok.text(SM)),
+ DefLoc},
+ include_cleaner::RefType::Explicit});
+ }
+ return Macros;
+}
+
include_cleaner::Includes
convertIncludes(const SourceManager &SM,
const llvm::ArrayRef<Inclusion> Includes) {
diff --git a/clang-tools-extra/clangd/IncludeCleaner.h b/clang-tools-extra/clangd/IncludeCleaner.h
index 1a5f07869d569..035142c186b80 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.h
+++ b/clang-tools-extra/clangd/IncludeCleaner.h
@@ -78,6 +78,9 @@ convertIncludes(const SourceManager &SM,
/// representation. The spelling contains the ""<> characters.
std::string spellHeader(ParsedAST &AST, const FileEntry *MainFile,
include_cleaner::Header Provider);
+
+std::vector<include_cleaner::SymbolReference>
+collectMacroReferences(ParsedAST &AST);
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index df46e2fa09951..36d73481809ed 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -10,6 +10,7 @@
#include "Annotations.h"
#include "Config.h"
#include "Hover.h"
+#include "TestFS.h"
#include "TestIndex.h"
#include "TestTU.h"
#include "index/MemIndex.h"
@@ -2980,6 +2981,91 @@ TEST(Hover, ParseProviderInfo) {
EXPECT_EQ(Case.HI.present().asMarkdown(), Case.ExpectedMarkdown);
}
+TEST(Hover, UsedSymbols) {
+ struct {
+ const char *Code;
+ const std::function<void(HoverInfo &)> ExpectedBuilder;
+ } Cases[] = {{R"cpp(
+ #include ^"bar.h"
+ int fstBar = bar1();
+ int sndBar = bar2();
+ Bar bar;
+ int macroBar = BAR;
+ )cpp",
+ [](HoverInfo &HI) {
+ HI.UsedSymbolNames = {"BAR", "Bar", "bar1", "bar2"};
+ }},
+ {R"cpp(
+ #in^clude <vector>
+ std::vector<int> vec;
+ )cpp",
+ [](HoverInfo &HI) { HI.UsedSymbolNames = {"vector"}; }},
+ {R"cpp(
+ #in^clude "public.h"
+ #include "private.h"
+ int fooVar = foo();
+ )cpp",
+ [](HoverInfo &HI) { HI.UsedSymbolNames = {"foo"}; }},
+ {R"cpp(
+ #include "bar.h"
+ #include "for^ward.h"
+ Bar *x;
+ )cpp",
+ [](HoverInfo &HI) {
+ HI.UsedSymbolNames = {
+ // No used symbols, since bar.h is a higher-ranked
+ // provider for Bar.
+ };
+ }},
+ {R"cpp(
+ #include "b^ar.h"
+ #define DEF(X) const Bar *X
+ DEF(a);
+ )cpp",
+ [](HoverInfo &HI) { HI.UsedSymbolNames = {"Bar"}; }},
+ {R"cpp(
+ #in^clude "bar.h"
+ #define BAZ(X) const X x
+ BAZ(Bar);
+ )cpp",
+ [](HoverInfo &HI) { HI.UsedSymbolNames = {"Bar"}; }}};
+ for (const auto &Case : Cases) {
+ Annotations Code{Case.Code};
+ SCOPED_TRACE(Code.code());
+
+ TestTU TU;
+ TU.Filename = "foo.cpp";
+ TU.Code = Code.code();
+ TU.AdditionalFiles["bar.h"] = guard(R"cpp(
+ #define BAR 5
+ int bar1();
+ int bar2();
+ class Bar {};
+ )cpp");
+ TU.AdditionalFiles["private.h"] = guard(R"cpp(
+ // IWYU pragma: private, include "public.h"
+ int foo();
+ )cpp");
+ TU.AdditionalFiles["public.h"] = guard("");
+ TU.AdditionalFiles["system/vector"] = guard(R"cpp(
+ namespace std {
+ template<typename>
+ class vector{};
+ }
+ )cpp");
+ TU.AdditionalFiles["forward.h"] = guard("class Bar;");
+ TU.ExtraArgs.push_back("-isystem" + testPath("system"));
+
+ auto AST = TU.build();
+ auto H = getHover(AST, Code.point(), format::getLLVMStyle(), nullptr);
+ ASSERT_TRUE(H);
+ HoverInfo Expected;
+ Case.ExpectedBuilder(Expected);
+ SCOPED_TRACE(H->present().asMarkdown());
+ EXPECT_EQ(H->UsedSymbolNames, Expected.UsedSymbolNames);
+ }
+}
+
TEST(Hover, DocsFromIndex) {
Annotations T(R"cpp(
template <typename T> class X {};
@@ -3369,7 +3455,21 @@ int foo = 3)",
R"(stdio.h
/usr/include/stdio.h)",
- }};
+ },
+ {[](HoverInfo &HI) {
+ HI.Name = "foo.h";
+ HI.UsedSymbolNames = {"Foo", "Bar", "Baz"};
+ },
+ R"(foo.h
+
+provides Foo, Bar, Baz)"},
+ {[](HoverInfo &HI) {
+ HI.Name = "foo.h";
+ HI.UsedSymbolNames = {"Foo", "Bar", "Baz", "Foobar", "Qux", "Quux"};
+ },
+ R"(foo.h
+
+provides Foo, Bar, Baz, Foobar, Qux and 1 more)"}};
for (const auto &C : Cases) {
HoverInfo HI;
More information about the cfe-commits
mailing list