VitaNuo updated this revision to Diff 504720.
VitaNuo added a comment.
Rebase to main.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144976/new/
https://reviews.llvm.org/D144976
Files:
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/Preamble.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -14,11 +14,12 @@
#include "TestTU.h"
#include "index/MemIndex.h"
#include "clang/AST/Attr.h"
+#include "clang/Format/Format.h"
#include "clang/Index/IndexSymbol.h"
#include "llvm/ADT/StringRef.h"
-#include "gmock/gmock.h"
#include "gtest/gtest.h"
+#include <functional>
#include <string>
#include <vector>
@@ -28,6 +29,10 @@
using PassMode = HoverInfo::PassType::PassMode;
+std::string guard(llvm::StringRef Code) {
+ return "#pragma once\n" + Code.str();
+}
+
TEST(Hover, Structured) {
struct {
const char *const Code;
@@ -2882,6 +2887,265 @@
}
}
+TEST(Hover, Providers) {
+ struct {
+ Annotations Code;
+ llvm::StringLiteral FooHeader;
+ llvm::StringLiteral BarHeader;
+ llvm::StringLiteral FooBarHeader;
+ const std::function<void(HoverInfo &)> ExpectedBuilder;
+ } Cases[] = {
+ {Annotations(
+ R"cpp(
+ struct Foo {};
+ Foo F = [[Fo^o]]{};
+ )cpp"),
+ "int foo();", "", "", [](HoverInfo &HI) { HI.Provider = ""; }},
+ {Annotations(
+ R"cpp(
+ #include "foo.h"
+ int F = [[fo^o]]();
+ )cpp"),
+ "int foo();", "", "", [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }},
+ {Annotations(
+ R"cpp(
+ #include "bar.h"
+ #include "foo.h"
+ int F = [[f^oo]]();
+ )cpp"),
+ "int foo();", "int foo();", "",
+ [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }},
+ {Annotations(
+ R"cpp(
+ #include "bar.h"
+ #include "foo.h"
+ int F = [[^foo]]();
+ )cpp"),
+ "int foo();", "#include \"foobar.h\"", "int foo();",
+ [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }},
+ {Annotations(
+ R"cpp(
+ #include "bar.h"
+ #include "foo.h"
+ #include "foobar.h"
+ int F = [[^foo]]();
+ )cpp"),
+ "int foo();", "int foo();", "#include \"bar.h\"",
+ [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }},
+ {Annotations(
+ R"cpp(
+ #include "bar.h"
+ #include "foo.h"
+ #include "foobar.h"
+ int F = [[foo^]]();
+ )cpp"),
+ "int foo();", "#include \"foo.h\"", "#include \"foo.h\"",
+ [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }},
+ {Annotations(
+ R"cpp(
+ #include "foo.h"
+
+ Foo [[f^oo]] = 42;
+ )cpp"),
+ R"cpp(
+ class Foo {
+ public:
+ Foo(int val): val(val) {}
+ private:
+ int val;
+ };
+ )cpp", "", "", [](HoverInfo &HI) { HI.Provider = ""; }},
+ {Annotations(
+ R"cpp(
+ #include "bar.h"
+
+ int F = [[f^oo]]();
+ )cpp"),
+ R"cpp(
+ int foo();
+ )cpp", "#include \"foo.h\"", "", [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }},
+ {Annotations(
+ R"cpp(
+ #include "bar.h"
+
+ int F = [[f^oo]]();
+ )cpp"),
+ "int foo();",
+ R"cpp(
+ #include "foo.h"
+ #include "foobar.h"
+
+ )cpp", "int foo();", [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }},
+ {Annotations(
+ R"cpp(
+ #include "bar.h"
+
+ int F = [[f^oo]]();
+ )cpp"),
+ R"cpp(
+ #include "foobar.h"
+ )cpp", R"cpp(
+ #include "foo.h"
+ #include "foobar.h"
+
+ )cpp", "int foo();", [](HoverInfo &HI) { HI.Provider = "\"foobar.h\""; }},
+ {Annotations(
+ R"cpp(
+ #include "foo.h"
+ int F = [[^MACRO]];
+ )cpp"),
+ "#define MACRO 5",
+ "", "",
+ [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }},
+ {Annotations(
+ R"cpp(
+ #include "bar.h"
+ #include "foo.h"
+
+ int F = [[M^ACRO]];
+ )cpp"),
+ "#define MACRO 5",
+ "#define MACRO 10", "",
+ [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }},
+ {Annotations(
+ R"cpp(
+ #include "bar.h"
+ #include "foo.h"
+
+ int F = [[MA^CRO]](5);
+ )cpp"),
+ "#define MACRO(X) X+1",
+ "#include \"foobar.h\"", "#define MACRO(X) X+3",
+ [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }},
+ {Annotations(
+ R"cpp(
+ #include "foo.h"
+ int [[^F]] = MACRO(5);
+ )cpp"),
+ "#define MACRO(X) (X+1)",
+ "", "",
+ [](HoverInfo &HI) { HI.Provider = ""; }},
+ {Annotations(
+ R"cpp(
+ #include "bar.h"
+ int F = [[MACR^O]];
+ )cpp"),
+ "#define MACRO 5", "#include \"foo.h\"", "",
+ [](HoverInfo &HI) { HI.Provider = "\"foo.h\""; }},
+ {Annotations(
+ R"cpp(
+ #include "bar.h"
+ int F = [[MACR^O]];
+ )cpp"),
+ "#define MACRO 5", R"cpp(
+ #include "foo.h"
+ #include "foobar.h"
+ )cpp", "#define MACRO 10",
+ [](HoverInfo &HI) { HI.Provider = "\"foobar.h\""; }},
+ {Annotations(
+ R"cpp(
+ #include "bar.h"
+
+ int F = [[fo^o]]();
+ )cpp"),
+ "", R"cpp(
+ // IWYU pragma: private, include "foo.h"
+ int foo();
+ )cpp", "",
+ [](HoverInfo &HI) { HI.Provider = "\"bar.h\""; }},
+ {Annotations(
+ R"cpp(
+ #include "bar.h"
+
+ int F = [[fo^o]]();
+ )cpp"),
+ "", "#include \"foobar.h\"",
+ R"cpp(
+ // IWYU pragma: private, include "public.h"
+ int foo();
+ )cpp",
+ [](HoverInfo &HI) { HI.Provider = "\"public.h\""; }},
+ {Annotations(
+ R"cpp(
+ #include "bar.h"
+
+ std::[[vect^or]] v;
+ )cpp"),
+ R"cpp(
+ namespace std {
+ class vector {};
+ }
+ )cpp", "#include \"foo.h\"",
+ "",
+ [](HoverInfo &HI) { HI.Provider = "<vector>"; }}
+ };
+
+ for (const auto &Case : Cases) {
+ SCOPED_TRACE(Case.Code.code());
+
+ TestTU TU;
+ TU.Filename = "foo.cpp";
+ TU.Code = Case.Code.code();
+ TU.AdditionalFiles["foo.h"] = guard(Case.FooHeader);
+ TU.AdditionalFiles["bar.h"] = guard(Case.BarHeader);
+ TU.AdditionalFiles["foobar.h"] = guard(Case.FooBarHeader);
+
+ auto AST = TU.build();
+ auto H = getHover(AST, Case.Code.point(), format::getLLVMStyle(), nullptr);
+ ASSERT_TRUE(H);
+ HoverInfo Expected;
+ Case.ExpectedBuilder(Expected);
+ SCOPED_TRACE(H->present().asMarkdown());
+ EXPECT_EQ(H->Provider, Expected.Provider);
+ }
+}
+
+TEST(Hover, ParseProviderInfo) {
+ HoverInfo HIFoo;
+ HIFoo.Name = "foo";
+ HIFoo.Provider = "foo.h";
+
+ HoverInfo HIFooBar;
+ HIFooBar.Name = "foo";
+ HIFooBar.Provider = "bar.h";
+ struct Case {
+ HoverInfo HI;
+ llvm::StringRef ExpectedMarkdown;
+ } Cases[] = {{HIFoo, "### `foo` \nprovided by `foo.h`"},
+ {HIFooBar, "### `foo` \nprovided by `bar.h`"}};
+
+ for (const auto &Case : Cases)
+ EXPECT_EQ(Case.HI.present().asMarkdown(), Case.ExpectedMarkdown);
+}
+
+TEST(Hover, NoProviderIfOtherDeclUsedForHoverCard) {
+ Annotations Code(R"cpp(
+ #include "absl_string_view.h"
+ absl::str^ing_view abc; // hover on string_view
+ )cpp");
+ llvm::StringLiteral AbslHeader = R"cpp(
+ #include "std_string_view.h"
+ namespace absl {
+ using std::string_view;
+ })cpp";
+ llvm::StringLiteral StdHeader = R"cpp(
+ namespace std {
+ class string_view {};
+ })cpp";
+
+ TestTU TU;
+ TU.Filename = "main.cpp";
+ TU.Code = Code.code();
+ TU.AdditionalFiles["absl_string_view.h"] = guard(AbslHeader);
+ TU.AdditionalFiles["std_string_view.h"] = guard(StdHeader);
+
+ auto AST = TU.build();
+ auto H = getHover(AST, Code.point(), format::getLLVMStyle(), nullptr);
+ ASSERT_TRUE(H);
+ ASSERT_TRUE(H->Provider.empty());
+}
+
+
TEST(Hover, DocsFromIndex) {
Annotations T(R"cpp(
template <typename T> class X {};
@@ -3359,8 +3623,8 @@
}
}
-// This is a separate test as headings don't create any differences in plaintext
-// mode.
+// This is a separate test as headings don't create any differences in
+// plaintext mode.
TEST(Hover, PresentHeadings) {
HoverInfo HI;
HI.Kind = index::SymbolKind::Variable;
Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -127,11 +127,7 @@
LangOpts = &CI.getLangOpts();
SourceMgr = &CI.getSourceManager();
Includes.collect(CI);
- if (Config::current().Diagnostics.UnusedIncludes ==
- Config::IncludesPolicy::Strict ||
- Config::current().Diagnostics.MissingIncludes ==
- Config::IncludesPolicy::Strict)
- Pragmas.record(CI);
+ Pragmas.record(CI);
if (BeforeExecuteCallback)
BeforeExecuteCallback(CI);
}
Index: clang-tools-extra/clangd/IncludeCleaner.h
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -68,6 +68,12 @@
/// FIXME: remove this hack once the implementation is good enough.
void setIncludeCleanerAnalyzesStdlib(bool B);
+include_cleaner::Includes
+convertIncludes(const SourceManager &SM,
+ const llvm::ArrayRef<Inclusion> MainFileIncludes);
+
+std::string spellHeader(ParsedAST &AST, const FileEntry *MainFile,
+ include_cleaner::Header Provider);
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -124,66 +124,6 @@
return true;
}
-include_cleaner::Includes
-convertIncludes(const SourceManager &SM,
- const llvm::ArrayRef<Inclusion> MainFileIncludes) {
- include_cleaner::Includes Includes;
- for (const Inclusion &Inc : MainFileIncludes) {
- include_cleaner::Include TransformedInc;
- llvm::StringRef WrittenRef = llvm::StringRef(Inc.Written);
- TransformedInc.Spelled = WrittenRef.trim("\"<>");
- TransformedInc.HashLocation =
- SM.getComposedLoc(SM.getMainFileID(), Inc.HashOffset);
- TransformedInc.Line = Inc.HashLine + 1;
- TransformedInc.Angled = WrittenRef.starts_with("<");
- auto FE = SM.getFileManager().getFile(Inc.Resolved);
- if (!FE) {
- elog("IncludeCleaner: Failed to get an entry for resolved path {0}: {1}",
- Inc.Resolved, FE.getError().message());
- continue;
- }
- TransformedInc.Resolved = *FE;
- Includes.add(std::move(TransformedInc));
- }
- return Includes;
-}
-
-std::string spellHeader(ParsedAST &AST, const FileEntry *MainFile,
- include_cleaner::Header Provider) {
- if (Provider.kind() == include_cleaner::Header::Physical) {
- if (auto CanonicalPath =
- getCanonicalPath(Provider.physical(), AST.getSourceManager())) {
- std::string SpelledHeader =
- llvm::cantFail(URI::includeSpelling(URI::create(*CanonicalPath)));
- if (!SpelledHeader.empty())
- return SpelledHeader;
- }
- }
- return include_cleaner::spellHeader(
- Provider, AST.getPreprocessor().getHeaderSearchInfo(), MainFile);
-}
-
-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:
@@ -313,8 +253,67 @@
}
return Result;
}
+
+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;
+}
} // namespace
+include_cleaner::Includes
+convertIncludes(const SourceManager &SM,
+ const llvm::ArrayRef<Inclusion> MainFileIncludes) {
+ include_cleaner::Includes Includes;
+ for (const Inclusion &Inc : MainFileIncludes) {
+ include_cleaner::Include TransformedInc;
+ llvm::StringRef WrittenRef = llvm::StringRef(Inc.Written);
+ TransformedInc.Spelled = WrittenRef.trim("\"<>");
+ TransformedInc.HashLocation =
+ SM.getComposedLoc(SM.getMainFileID(), Inc.HashOffset);
+ TransformedInc.Line = Inc.HashLine + 1;
+ TransformedInc.Angled = WrittenRef.starts_with("<");
+ auto FE = SM.getFileManager().getFile(Inc.Resolved);
+ if (!FE) {
+ elog("IncludeCleaner: Failed to get an entry for resolved path {0}: {1}",
+ Inc.Resolved, FE.getError().message());
+ continue;
+ }
+ TransformedInc.Resolved = *FE;
+ Includes.add(std::move(TransformedInc));
+ }
+ return Includes;
+}
+
+std::string spellHeader(ParsedAST &AST, const FileEntry *MainFile,
+ include_cleaner::Header Provider) {
+ if (Provider.kind() == include_cleaner::Header::Physical) {
+ if (auto CanonicalPath =
+ getCanonicalPath(Provider.physical(), AST.getSourceManager())) {
+ std::string SpelledHeader =
+ llvm::cantFail(URI::includeSpelling(URI::create(*CanonicalPath)));
+ if (!SpelledHeader.empty())
+ return SpelledHeader;
+ }
+ }
+ return include_cleaner::spellHeader(
+ Provider, AST.getPreprocessor().getHeaderSearchInfo(), MainFile);
+}
std::vector<const Inclusion *>
getUnused(ParsedAST &AST,
Index: clang-tools-extra/clangd/Hover.h
===================================================================
--- clang-tools-extra/clangd/Hover.h
+++ clang-tools-extra/clangd/Hover.h
@@ -14,6 +14,7 @@
#include "support/Markup.h"
#include "clang/Index/IndexSymbol.h"
#include <optional>
+#include <string>
namespace clang {
namespace clangd {
@@ -67,6 +68,8 @@
std::string LocalScope;
/// Name of the symbol, does not contain any "::".
std::string Name;
+ /// Header providing the symbol (best match).
+ std::string Provider;
std::optional<Range> SymRange;
index::SymbolKind Kind = index::SymbolKind::Unknown;
std::string Documentation;
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -12,11 +12,16 @@
#include "CodeCompletionStrings.h"
#include "Config.h"
#include "FindTarget.h"
+#include "IncludeCleaner.h"
#include "ParsedAST.h"
#include "Selection.h"
#include "SourceCode.h"
+#include "clang-include-cleaner/Analysis.h"
+#include "clang-include-cleaner/Types.h"
#include "index/SymbolCollector.h"
+#include "support/Logger.h"
#include "support/Markup.h"
+#include "support/Trace.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTDiagnostic.h"
#include "clang/AST/ASTTypeTraits.h"
@@ -43,11 +48,13 @@
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/Format.h"
#include "llvm/Support/ScopedPrinter.h"
#include "llvm/Support/raw_ostream.h"
#include <optional>
#include <string>
+#include <vector>
namespace clang {
namespace clangd {
@@ -1084,6 +1091,71 @@
return Candidates.front();
}
+void maybeAddSymbolProviders(
+ ParsedAST &AST, HoverInfo &HI, std::optional<const NamedDecl *> UsedDecl,
+ SourceLocation CurLoc,
+ std::vector<include_cleaner::SymbolReference> MacroReferences) {
+ trace::Span Tracer("Hover::maybeAddSymbolProviders");
+ include_cleaner::walkUsed(
+ AST.getLocalTopLevelDecls(), MacroReferences, AST.getPragmaIncludes(),
+ AST.getSourceManager(),
+ [&](const include_cleaner::SymbolReference &Ref,
+ llvm::ArrayRef<include_cleaner::Header> Providers) {
+ if (Providers.empty() || !Ref.RefLocation.isFileID())
+ // Ignore symbols with no providers and locations within macro
+ // expansions
+ return;
+
+ const SourceManager &SM = AST.getSourceManager();
+ const syntax::Token *Tok =
+ AST.getTokens().spelledTokenAt(Ref.RefLocation);
+ if (Tok == nullptr) {
+ elog("Can't find spelled token at the symbol reference location.");
+ return;
+ }
+
+ // Check that the user is hovering over the current symbol reference.
+ if (CurLoc < Ref.RefLocation || CurLoc > Tok->endLocation())
+ return;
+
+ if (Ref.Target.kind() == include_cleaner::Symbol::Kind::Declaration &&
+ UsedDecl &&
+ Ref.Target.declaration().getCanonicalDecl() !=
+ (*UsedDecl)->getCanonicalDecl())
+ // Make sure we only pick the provider if the reference is
+ // for the same decl as is used for the rest of the hover card.
+ return;
+
+ std::string Result;
+ include_cleaner::Includes ConvertedIncludes =
+ convertIncludes(SM, AST.getIncludeStructure().MainFileIncludes);
+ for (const auto &P : Providers) {
+ auto Matches = ConvertedIncludes.match(P);
+ if (!Matches.empty()) {
+ // Pick the best-ranked included provider
+ Result = Matches[0]->quote();
+ break;
+ }
+ }
+
+ if (!Result.empty()) {
+ HI.Provider = std::move(Result);
+ return;
+ }
+
+ for (const auto &P : Providers) {
+ if (P.kind() == include_cleaner::Header::Physical &&
+ P.physical() == SM.getFileEntryForID(SM.getMainFileID()))
+ continue;
+
+ // Pick the best-ranked non-included provider
+ HI.Provider =
+ spellHeader(AST, SM.getFileEntryForID(SM.getMainFileID()), P);
+ break;
+ }
+ });
+}
+
} // namespace
std::optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
@@ -1131,6 +1203,15 @@
HighlightRange = Tok.range(SM).toCharRange(SM);
if (auto M = locateMacroAt(Tok, AST.getPreprocessor())) {
HI = getHoverContents(*M, Tok, AST);
+ if (auto DefLoc = M->Info->getDefinitionLoc(); DefLoc.isValid()) {
+ include_cleaner::SymbolReference MacroSymRef{
+ Tok.location(),
+ include_cleaner::Macro{
+ AST.getPreprocessor().getIdentifierInfo(Tok.text(SM)),
+ DefLoc},
+ include_cleaner::RefType::Explicit};
+ maybeAddSymbolProviders(AST, *HI, {}, *CurLoc, {MacroSymRef});
+ }
break;
}
} else if (Tok.kind() == tok::kw_auto || Tok.kind() == tok::kw_decltype) {
@@ -1168,6 +1249,7 @@
if (!HI->Value)
HI->Value = printExprValue(N, AST.getASTContext());
maybeAddCalleeArgInfo(N, *HI, PP);
+ maybeAddSymbolProviders(AST, *HI, DeclToUse, *CurLoc, {});
} else if (const Expr *E = N->ASTNode.get<Expr>()) {
HI = getHoverContents(N, E, AST, PP, Index);
} else if (const Attr *A = N->ASTNode.get<Attr>()) {
@@ -1217,6 +1299,14 @@
assert(!Name.empty() && "hover triggered on a nameless symbol");
Header.appendCode(Name);
+ if (!Provider.empty()) {
+ markup::Paragraph &DI = Output.addParagraph();
+ DI.appendText("provided by");
+ DI.appendSpace();
+ DI.appendCode(Provider);
+ Output.addRuler();
+ }
+
// Put a linebreak after header to increase readability.
Output.addRuler();
// Print Types on their own lines to reduce chances of getting line-wrapped by
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
