kadircet updated this revision to Diff 198295.
kadircet marked 8 inline comments as done.
kadircet added a comment.
- Address comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61497/new/
https://reviews.llvm.org/D61497
Files:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/XRefs.h
clang-tools-extra/clangd/unittests/XRefsTests.cpp
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1185,7 +1185,9 @@
auto AST = TU.build();
if (auto H = getHover(AST, T.point())) {
EXPECT_NE("", Test.ExpectedHover) << Test.Input;
- EXPECT_EQ(H->contents.value, Test.ExpectedHover.str()) << Test.Input;
+ EXPECT_EQ(H->render().contents.value, Test.ExpectedHover.str())
+ << Test.Input << '\n'
+ << H;
} else
EXPECT_EQ("", Test.ExpectedHover.str()) << Test.Input;
}
Index: clang-tools-extra/clangd/XRefs.h
===================================================================
--- clang-tools-extra/clangd/XRefs.h
+++ clang-tools-extra/clangd/XRefs.h
@@ -16,7 +16,10 @@
#include "ClangdUnit.h"
#include "Protocol.h"
#include "index/Index.h"
+#include "index/SymbolLocation.h"
+#include "clang/Index/IndexSymbol.h"
#include "llvm/ADT/Optional.h"
+#include "llvm/Support/raw_ostream.h"
#include <vector>
namespace clang {
@@ -46,8 +49,37 @@
std::vector<DocumentHighlight> findDocumentHighlights(ParsedAST &AST,
Position Pos);
+struct HoverInfo {
+ using Type = std::string;
+ struct Param {
+ Type T;
+ std::string Name;
+ std::string Default;
+ };
+
+ LocatedSymbol Sym;
+ /// Fully qualiffied name for the scope containing the Sym.
+ std::string Scope;
+ std::string ParentScope;
+ SymbolKind Kind;
+ std::string Documentation;
+ /// Line containing the definition of the symbol.
+ std::string Definition;
+
+ /// T and ReturnType are mutually exclusive.
+ llvm::Optional<Type> T;
+ llvm::Optional<Type> ReturnType;
+ llvm::Optional<std::vector<Param>> Parameters;
+ llvm::Optional<std::vector<Param>> TemplateParameters;
+
+ /// Lower to LSP struct.
+ Hover render() const;
+};
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, const HoverInfo::Param &);
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, const HoverInfo &);
+
/// Get the hover information when hovering at \p Pos.
-llvm::Optional<Hover> getHover(ParsedAST &AST, Position Pos);
+llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos);
/// Returns reference locations of the symbol at a specified \p Pos.
/// \p Limit limits the number of results returned (0 means no limit).
Index: clang-tools-extra/clangd/XRefs.cpp
===================================================================
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -7,21 +7,32 @@
//===----------------------------------------------------------------------===//
#include "XRefs.h"
#include "AST.h"
+#include "CodeCompletionStrings.h"
#include "FindSymbols.h"
#include "Logger.h"
+#include "Protocol.h"
#include "SourceCode.h"
#include "URI.h"
#include "index/Merge.h"
#include "index/SymbolCollector.h"
#include "index/SymbolLocation.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
#include "clang/AST/DeclTemplate.h"
+#include "clang/AST/PrettyPrinter.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/AST/Type.h"
+#include "clang/Basic/SourceLocation.h"
#include "clang/Index/IndexDataConsumer.h"
#include "clang/Index/IndexSymbol.h"
#include "clang/Index/IndexingAction.h"
#include "clang/Index/USRGeneration.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
namespace clang {
namespace clangd {
@@ -241,17 +252,17 @@
return {DeclMacrosFinder.getFoundDecls(), DeclMacrosFinder.takeMacroInfos()};
}
-Range getTokenRange(ParsedAST &AST, SourceLocation TokLoc) {
- const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
- SourceLocation LocEnd = Lexer::getLocForEndOfToken(
- TokLoc, 0, SourceMgr, AST.getASTContext().getLangOpts());
+Range getTokenRange(ASTContext &AST, SourceLocation TokLoc) {
+ const SourceManager &SourceMgr = AST.getSourceManager();
+ SourceLocation LocEnd =
+ Lexer::getLocForEndOfToken(TokLoc, 0, SourceMgr, AST.getLangOpts());
return {sourceLocToPosition(SourceMgr, TokLoc),
sourceLocToPosition(SourceMgr, LocEnd)};
}
-llvm::Optional<Location> makeLocation(ParsedAST &AST, SourceLocation TokLoc,
+llvm::Optional<Location> makeLocation(ASTContext &AST, SourceLocation TokLoc,
llvm::StringRef TUPath) {
- const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
+ const SourceManager &SourceMgr = AST.getSourceManager();
const FileEntry *F = SourceMgr.getFileEntryForID(SourceMgr.getFileID(TokLoc));
if (!F)
return None;
@@ -299,8 +310,8 @@
// As a consequence, there's no need to look them up in the index either.
std::vector<LocatedSymbol> Result;
for (auto M : Symbols.Macros) {
- if (auto Loc =
- makeLocation(AST, M.Info->getDefinitionLoc(), *MainFilePath)) {
+ if (auto Loc = makeLocation(AST.getASTContext(), M.Info->getDefinitionLoc(),
+ *MainFilePath)) {
LocatedSymbol Macro;
Macro.Name = M.Name;
Macro.PreferredDeclaration = *Loc;
@@ -320,7 +331,7 @@
// Emit all symbol locations (declaration or definition) from AST.
for (const Decl *D : Symbols.Decls) {
- auto Loc = makeLocation(AST, findNameLoc(D), *MainFilePath);
+ auto Loc = makeLocation(AST.getASTContext(), findNameLoc(D), *MainFilePath);
if (!Loc)
continue;
@@ -453,7 +464,7 @@
std::vector<DocumentHighlight> Result;
for (const auto &Ref : References) {
DocumentHighlight DH;
- DH.range = getTokenRange(AST, Ref.Loc);
+ DH.range = getTokenRange(AST.getASTContext(), Ref.Loc);
if (Ref.Role & index::SymbolRoleSet(index::SymbolRole::Write))
DH.kind = DocumentHighlightKind::Write;
else if (Ref.Role & index::SymbolRoleSet(index::SymbolRole::Read))
@@ -525,54 +536,206 @@
return None;
}
-/// Generate a \p Hover object given the declaration \p D.
-static Hover getHoverContents(const Decl *D) {
- Hover H;
- llvm::Optional<std::string> NamedScope = getScopeName(D);
+static QualType getDeclType(const Decl *D) {
+ if (const TypedefNameDecl *TDD = dyn_cast<TypedefNameDecl>(D))
+ return TDD->getUnderlyingType();
+ if (const ValueDecl *VD = dyn_cast<ValueDecl>(D))
+ return VD->getType();
+ if (const TagDecl *TD = dyn_cast<TagDecl>(D))
+ return TD->getTypeForDecl()->getCanonicalTypeInternal();
+ return QualType();
+}
- // Generate the "Declared in" section.
- if (NamedScope) {
- assert(!NamedScope->empty());
+static llvm::Optional<Location> getDeclLoc(const SourceLocation &SL,
+ ASTContext &AST) {
+ if (SL.isInvalid())
+ return llvm::None;
+ const auto &SM = AST.getSourceManager();
+ auto MainFilePath =
+ getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
+ if (!MainFilePath) {
+ elog("Couldn't get main file path for: {0}", SL.printToString(SM));
+ return llvm::None;
+ }
+ return makeLocation(AST, SL, *MainFilePath);
+}
- H.contents.value += "Declared in ";
- H.contents.value += *NamedScope;
- H.contents.value += "\n\n";
+static std::string getDefinitionLine(const Decl *D) {
+ std::string Definition;
+ {
+ llvm::raw_string_ostream OS(Definition);
+ PrintingPolicy Policy =
+ printingPolicyForDecls(D->getASTContext().getPrintingPolicy());
+ D->print(OS, Policy);
}
+ return Definition;
+}
- // We want to include the template in the Hover.
- if (TemplateDecl *TD = D->getDescribedTemplate())
- D = TD;
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
+ const std::vector<HoverInfo::Param> &Params) {
+ for (size_t I = 0, E = Params.size(); I != E; ++I) {
+ if (I)
+ OS << ", ";
+ OS << Params.at(I);
+ }
+ return OS;
+}
- std::string DeclText;
- llvm::raw_string_ostream OS(DeclText);
+static std::vector<HoverInfo::Param>
+fetchTemplateParameters(const TemplateParameterList *Params,
+ const PrintingPolicy &PP) {
+ assert(Params);
+ std::vector<HoverInfo::Param> TempParameters;
+
+ for (const Decl *Param : *Params) {
+ HoverInfo::Param P;
+ if (const auto TTP = dyn_cast<TemplateTypeParmDecl>(Param)) {
+ P.T = TTP->wasDeclaredWithTypename() ? "typename" : "class";
+ if (TTP->isParameterPack())
+ P.T += "...";
+
+ llvm::raw_string_ostream Out(P.Name);
+ Out << *TTP;
+ if (TTP->hasDefaultArgument())
+ P.Default = TTP->getDefaultArgument().getAsString(PP);
+ } else if (const auto NTTP = dyn_cast<NonTypeTemplateParmDecl>(Param)) {
+ if (IdentifierInfo *II = NTTP->getIdentifier())
+ P.Name = II->getName().str();
+
+ llvm::raw_string_ostream Out(P.T);
+ NTTP->getType().print(Out, PP);
+ if (NTTP->isParameterPack())
+ Out << "...";
+
+ if (NTTP->hasDefaultArgument()) {
+ llvm::raw_string_ostream Out(P.Default);
+ NTTP->getDefaultArgument()->printPretty(Out, nullptr, PP);
+ }
+ } else if (const auto TTPD = dyn_cast<TemplateTemplateParmDecl>(Param)) {
+ llvm::raw_string_ostream OS(P.T);
+ OS << "template <"
+ << fetchTemplateParameters(
+ TTPD->getDescribedTemplate()->getTemplateParameters(), PP)
+ << '>';
+ if (TTPD->hasDefaultArgument()) {
+ llvm::raw_string_ostream Out(P.Default);
+ TTPD->getDefaultArgument().getArgument().print(PP, Out);
+ }
+ } else {
+ std::string ParamSerial;
+ llvm::raw_string_ostream Out(ParamSerial);
+ Param->print(Out, PP);
+ Out.flush();
+ vlog("Unhandled template param: {0}", ParamSerial);
+ continue;
+ }
+ TempParameters.push_back(std::move(P));
+ }
+
+ return TempParameters;
+}
+
+/// Generate a \p Hover object given the declaration \p D.
+static HoverInfo getHoverContents(const Decl *D) {
+ HoverInfo HI;
PrintingPolicy Policy =
printingPolicyForDecls(D->getASTContext().getPrintingPolicy());
+ if (const NamedDecl *ND = llvm::dyn_cast<NamedDecl>(D)) {
+ HI.Documentation = getDeclComment(ND->getASTContext(), *ND);
+ HI.Sym.Name = printName(D->getASTContext(), *ND);
+ HI.Scope = printQualifiedName(*ND);
+ }
- D->print(OS, Policy);
+ if (llvm::Optional<std::string> NamedScope = getScopeName(D)) {
+ assert(!NamedScope->empty());
+ HI.ParentScope = std::move(*NamedScope);
+ }
+ HI.Kind = indexSymbolKindToSymbolKind(index::getSymbolInfo(D).Kind);
- OS.flush();
+ if (const TemplateDecl *TD = D->getDescribedTemplate()) {
+ HI.TemplateParameters =
+ fetchTemplateParameters(TD->getTemplateParameters(), Policy);
+ D = TD;
+ } else if (const FunctionDecl *FD = D->getAsFunction()) {
+ if (const auto FTD = FD->getDescribedTemplate()) {
+ HI.TemplateParameters =
+ fetchTemplateParameters(FTD->getTemplateParameters(), Policy);
+ D = FTD;
+ }
+ }
- H.contents.value += DeclText;
- return H;
+ if (const FunctionDecl *FD = D->getAsFunction()) {
+ HI.ReturnType.emplace();
+ llvm::raw_string_ostream OS(*HI.ReturnType);
+ FD->getReturnType().print(OS, Policy);
+
+ HI.Parameters.emplace();
+ for (const ParmVarDecl *PVD : FD->parameters()) {
+ HI.Parameters->emplace_back();
+ auto &P = HI.Parameters->back();
+ P.T = PVD->getNameAsString();
+ if (PVD->hasDefaultArg()) {
+ llvm::raw_string_ostream Out(P.Default);
+ PVD->getDefaultArg()->printPretty(Out, nullptr, Policy);
+ }
+ }
+ if (FD->isVariadic()) {
+ HI.Parameters->emplace_back();
+ HI.Parameters->back().T = "...";
+ }
+ } else {
+ auto QT = getDeclType(D);
+ if (!QT.isNull()) {
+ HI.T.emplace();
+ llvm::raw_string_ostream OS(*HI.T);
+ QT.print(OS, std::move(Policy));
+ }
+ }
+
+ if (auto DefLoc = getDeclLoc(findNameLoc(D), D->getASTContext())) {
+ HI.Sym.PreferredDeclaration = *DefLoc;
+ if (getDefinition(D) == D)
+ HI.Sym.Definition = *DefLoc;
+ }
+ HI.Definition = getDefinitionLine(D);
+ return HI;
}
/// Generate a \p Hover object given the type \p T.
-static Hover getHoverContents(QualType T, ASTContext &ASTCtx) {
- Hover H;
- std::string TypeText;
- llvm::raw_string_ostream OS(TypeText);
- PrintingPolicy Policy = printingPolicyForDecls(ASTCtx.getPrintingPolicy());
- T.print(OS, Policy);
- OS.flush();
- H.contents.value += TypeText;
- return H;
+static HoverInfo getHoverContents(QualType T, ASTContext &ASTCtx) {
+ HoverInfo HI;
+ {
+ HI.T.emplace();
+ llvm::raw_string_ostream OS(*HI.T);
+ PrintingPolicy Policy = printingPolicyForDecls(ASTCtx.getPrintingPolicy());
+ T.print(OS, Policy);
+ }
+ HI.Sym.Name = *HI.T;
+ if (const TagDecl *TD = T.getTypePtr()->getAsTagDecl()) {
+ if (auto ScopedName = getScopeName(TD))
+ HI.ParentScope = std::move(*ScopedName);
+ HI.Scope = printQualifiedName(*TD);
+ HI.Kind = indexSymbolKindToSymbolKind(index::getSymbolInfo(TD).Kind);
+ // FIXME: Populate documentation
+ if (auto DefLoc = getDeclLoc(findNameLoc(TD), TD->getASTContext())) {
+ HI.Sym.PreferredDeclaration = *DefLoc;
+ if (getDefinition(TD) == TD)
+ HI.Sym.Definition = *DefLoc;
+ }
+ HI.Definition = getDefinitionLine(TD);
+ }
+ return HI;
}
/// Generate a \p Hover object given the macro \p MacroDecl.
-static Hover getHoverContents(MacroDecl Decl, ParsedAST &AST) {
+static HoverInfo getHoverContents(MacroDecl Decl, ParsedAST &AST) {
+ HoverInfo HI;
SourceManager &SM = AST.getASTContext().getSourceManager();
- std::string Definition = Decl.Name;
+ HI.Sym.Name = Decl.Name;
+ HI.Kind = indexSymbolKindToSymbolKind(
+ index::getSymbolInfoForMacro(*Decl.Info).Kind);
+ // FIXME: Populate documentation
// Try to get the full definition, not just the name
SourceLocation StartLoc = Decl.Info->getDefinitionLoc();
@@ -586,14 +749,15 @@
unsigned StartOffset = SM.getFileOffset(StartLoc);
unsigned EndOffset = SM.getFileOffset(EndLoc);
if (EndOffset <= Buffer.size() && StartOffset < EndOffset)
- Definition = Buffer.substr(StartOffset, EndOffset - StartOffset).str();
+ HI.Definition =
+ Buffer.substr(StartOffset, EndOffset - StartOffset).str();
}
}
-
- Hover H;
- H.contents.kind = MarkupKind::PlainText;
- H.contents.value = "#define " + Definition;
- return H;
+ if (auto DefLoc = getDeclLoc(StartLoc, AST.getASTContext())) {
+ HI.Sym.PreferredDeclaration = *DefLoc;
+ HI.Sym.Definition = *DefLoc;
+ }
+ return HI;
}
namespace {
@@ -708,7 +872,7 @@
return V.getDeducedType();
}
-llvm::Optional<Hover> getHover(ParsedAST &AST, Position Pos) {
+llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos) {
const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
SourceLocation SourceLocationBeg =
getBeginningOfIdentifier(AST, Pos, SourceMgr.getMainFileID());
@@ -748,7 +912,7 @@
auto MainFileRefs = findRefs(Symbols.Decls, AST);
for (const auto &Ref : MainFileRefs) {
Location Result;
- Result.range = getTokenRange(AST, Ref.Loc);
+ Result.range = getTokenRange(AST.getASTContext(), Ref.Loc);
Result.uri = URIForFile::canonicalize(*MainFilePath, *MainFilePath);
Results.push_back(std::move(Result));
}
@@ -991,5 +1155,70 @@
return Result;
}
+Hover HoverInfo::render() const {
+ Hover H;
+ H.range = Sym.PreferredDeclaration.range;
+ H.contents.kind = MarkupKind::PlainText;
+ std::vector<std::string> Output;
+
+ if (!Scope.empty())
+ H.contents.value = llvm::formatv("Declared in {0}\n\n", Scope);
+
+ if (Kind == SymbolKind::String) {
+ // We use SymbolKind::String for macro kinds
+ Output.push_back("#define");
+ if (!Definition.empty())
+ Output.push_back(Definition);
+ else
+ Output.push_back(Sym.Name);
+ } else if (T || ReturnType) {
+ // Non-macros
+ if (TemplateParameters) {
+ Output.emplace_back();
+ llvm::raw_string_ostream OS(Output.back());
+ OS << "template <" << *TemplateParameters << '>';
+ }
+ if (T)
+ Output.push_back(*T);
+ else if (ReturnType)
+ Output.push_back(*ReturnType);
+
+ if (!Sym.Name.empty())
+ if (!T || !llvm::StringRef(*T).endswith(Sym.Name))
+ Output.push_back(Sym.Name);
+
+ if (Parameters) {
+ // We don't want space before parameters.
+ llvm::raw_string_ostream OS(Output.back());
+ OS << '(' << *Parameters << ')';
+ }
+ } else {
+ // Symbols without types, like namespaces.
+ Output.push_back(Definition);
+ }
+ H.contents.value += llvm::join(Output, " ");
+ return H;
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
+ const HoverInfo::Param &P) {
+ OS << P.T << ' ' << P.Name;
+ if (!P.Default.empty())
+ OS << " = " << P.Default;
+ return OS;
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const HoverInfo &HI) {
+ OS << HI.Definition << '\n';
+ OS << HI.Documentation << '\n';
+ OS << HI.Parameters << '\n';
+ OS << HI.ReturnType << '\n';
+ OS << HI.Scope << '\n';
+ OS << HI.Sym << '\n';
+ OS << HI.T << '\n';
+ OS << HI.TemplateParameters << '\n';
+ return OS;
+}
+
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -180,7 +180,7 @@
/// Get code hover for a given position.
void findHover(PathRef File, Position Pos,
- Callback<llvm::Optional<Hover>> CB);
+ Callback<llvm::Optional<HoverInfo>> CB);
/// Get information about type hierarchy for a given position.
void typeHierarchy(PathRef File, Position Pos, int Resolve,
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -523,8 +523,8 @@
}
void ClangdServer::findHover(PathRef File, Position Pos,
- Callback<llvm::Optional<Hover>> CB) {
- auto Action = [Pos](Callback<llvm::Optional<Hover>> CB,
+ Callback<llvm::Optional<HoverInfo>> CB) {
+ auto Action = [Pos](Callback<llvm::Optional<HoverInfo>> CB,
llvm::Expected<InputsAndAST> InpAST) {
if (!InpAST)
return CB(InpAST.takeError());
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -839,7 +839,17 @@
void ClangdLSPServer::onHover(const TextDocumentPositionParams &Params,
Callback<llvm::Optional<Hover>> Reply) {
Server->findHover(Params.textDocument.uri.file(), Params.position,
- std::move(Reply));
+ Bind(
+ [](decltype(Reply) Reply,
+ llvm::Expected<llvm::Optional<HoverInfo>> HIorErr) {
+ if (!HIorErr)
+ return Reply(HIorErr.takeError());
+ const auto &HI = HIorErr.get();
+ if (!HI)
+ return Reply(llvm::None);
+ return Reply(HI->render());
+ },
+ std::move(Reply)));
}
void ClangdLSPServer::onTypeHierarchy(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
