[PATCH] D35894: [clangd] Code hover for Clangd
Marc-Andre Laperle via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 29 21:54:21 PST 2017
malaperle added inline comments.
================
Comment at: clangd/ClangdUnit.cpp:1117
+
+ if (!DeclVector.empty()) {
+ const Decl *LocationDecl = DeclVector[0];
----------------
malaperle wrote:
> malaperle wrote:
> > malaperle wrote:
> > > I think we need to simplify this part a bit. I'll try to find a way. Feel free to wait until more comments before updating.
> > Here's the version in which I tried to simplify this *a bit*. With the new ErrorOr checks as well.
> >
> > ```
> > Hover clangd::getHover(ParsedAST &AST, Position Pos, clangd::Logger &Logger) {
> > const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
> > const LangOptions &LangOpts = AST.getASTContext().getLangOpts();
> > const FileEntry *FE = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
> > if (!FE)
> > return Hover();
> >
> > SourceLocation SourceLocationBeg = getBeginningOfIdentifier(AST, Pos, FE);
> >
> > auto DeclMacrosFinder = std::make_shared<DeclarationAndMacrosFinder>(
> > llvm::errs(), SourceLocationBeg, AST.getASTContext(),
> > AST.getPreprocessor());
> > index::IndexingOptions IndexOpts;
> > IndexOpts.SystemSymbolFilter =
> > index::IndexingOptions::SystemSymbolFilterKind::All;
> > IndexOpts.IndexFunctionLocals = true;
> > indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
> > DeclMacrosFinder, IndexOpts);
> >
> > auto Decls = DeclMacrosFinder->takeDecls();
> > if (!Decls.empty() && Decls[0]) {
> > const Decl *LocationDecl = Decls[0];
> > std::vector<MarkedString> HoverContents;
> >
> > // Compute scope as the first "section" of the hover.
> > if (const NamedDecl *NamedD = dyn_cast<NamedDecl>(LocationDecl)) {
> > std::string Scope = getScope(NamedD, AST.getASTContext().getLangOpts());
> > if (!Scope.empty()) {
> > MarkedString Info = {"", "C++", "In " + Scope};
> > HoverContents.push_back(Info);
> > }
> > }
> >
> > // Adjust beginning of hover text depending on the presence of templates and comments.
> > TemplateDecl * TDec = LocationDecl->getDescribedTemplate();
> > const Decl * BeginDecl = TDec ? TDec : LocationDecl;
> > SourceLocation BeginLocation = BeginDecl->getSourceRange().getBegin();
> > RawComment *RC = AST.getASTContext().getRawCommentForDeclNoCache(
> > BeginDecl);
> > if (RC)
> > BeginLocation = RC->getLocStart();
> >
> > // Adjust end of hover text for things that have braces/bodies. We don't
> > // want to show the whole definition of a function, class, etc.
> > SourceLocation EndLocation = LocationDecl->getSourceRange().getEnd();
> > if (auto FuncDecl = dyn_cast<FunctionDecl>(LocationDecl)) {
> > if (FuncDecl->isThisDeclarationADefinition() && FuncDecl->getBody())
> > EndLocation = FuncDecl->getBody()->getLocStart();
> > } else if (auto TagDeclaration = dyn_cast<TagDecl>(LocationDecl)) {
> > if (TagDeclaration->isThisDeclarationADefinition())
> > EndLocation = TagDeclaration->getBraceRange().getBegin();
> > } else if (auto NameDecl = dyn_cast<NamespaceDecl>(LocationDecl)) {
> > SourceLocation BeforeLBraceLoc = Lexer::getLocForEndOfToken(
> > LocationDecl->getLocation(), 0, SourceMgr, LangOpts);
> > if (BeforeLBraceLoc.isValid())
> > EndLocation = BeforeLBraceLoc;
> > }
> >
> > SourceRange SR(BeginLocation, EndLocation);
> > if (SR.isValid()) {
> > auto L = getDeclarationLocation(AST, SR);
> > if (L) {
> > auto Ref = getBufferDataFromSourceRange(AST, *L);
> > if (Ref) {
> > Hover H;
> > if (SourceMgr.isInMainFile(BeginLocation))
> > H.range = L->range;
> > MarkedString MS = {"", "C++", *Ref};
> > HoverContents.push_back(MS);
> > H.contents = std::move(HoverContents);
> > return H;
> > }
> > }
> > }
> > }
> >
> > auto MacroInfos = DeclMacrosFinder->takeMacroInfos();
> > if (!MacroInfos.empty() && MacroInfos[0]) {
> > const MacroInfo *MacroInf = MacroInfos[0];
> > SourceRange SR(MacroInf->getDefinitionLoc(),
> > MacroInf->getDefinitionEndLoc());
> > if (SR.isValid()) {
> > auto L = getDeclarationLocation(AST, SR);
> > if (L) {
> > auto Ref = getBufferDataFromSourceRange(AST, *L);
> > if (Ref) {
> > Hover H;
> > if (SourceMgr.isInMainFile(SR.getBegin()))
> > H.range = L->range;
> > MarkedString MS = {"", "C++", "#define " + Ref->str()};
> > H.contents.push_back(MS);
> > return H;
> > }
> > }
> > }
> > }
> >
> > return Hover();
> > }
> > ```
> Here's the version in which I tried to simplify this *a bit*. With the new ErrorOr checks as well.
>
> ```
> Hover clangd::getHover(ParsedAST &AST, Position Pos, clangd::Logger &Logger) {
> const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
> const LangOptions &LangOpts = AST.getASTContext().getLangOpts();
> const FileEntry *FE = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
> if (!FE)
> return Hover();
>
> SourceLocation SourceLocationBeg = getBeginningOfIdentifier(AST, Pos, FE);
>
> auto DeclMacrosFinder = std::make_shared<DeclarationAndMacrosFinder>(
> llvm::errs(), SourceLocationBeg, AST.getASTContext(),
> AST.getPreprocessor());
> index::IndexingOptions IndexOpts;
> IndexOpts.SystemSymbolFilter =
> index::IndexingOptions::SystemSymbolFilterKind::All;
> IndexOpts.IndexFunctionLocals = true;
> indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
> DeclMacrosFinder, IndexOpts);
>
> auto Decls = DeclMacrosFinder->takeDecls();
> if (!Decls.empty() && Decls[0]) {
> const Decl *LocationDecl = Decls[0];
> std::vector<MarkedString> HoverContents;
>
> // Compute scope as the first "section" of the hover.
> if (const NamedDecl *NamedD = dyn_cast<NamedDecl>(LocationDecl)) {
> std::string Scope = getScope(NamedD, AST.getASTContext().getLangOpts());
> if (!Scope.empty()) {
> MarkedString Info = {"", "C++", "In " + Scope};
> HoverContents.push_back(Info);
> }
> }
>
> // Adjust beginning of hover text depending on the presence of templates and comments.
> TemplateDecl * TDec = LocationDecl->getDescribedTemplate();
> const Decl * BeginDecl = TDec ? TDec : LocationDecl;
> SourceLocation BeginLocation = BeginDecl->getSourceRange().getBegin();
> RawComment *RC = AST.getASTContext().getRawCommentForDeclNoCache(
> BeginDecl);
> if (RC)
> BeginLocation = RC->getLocStart();
>
> // Adjust end of hover text for things that have braces/bodies. We don't
> // want to show the whole definition of a function, class, etc.
> SourceLocation EndLocation = LocationDecl->getSourceRange().getEnd();
> if (auto FuncDecl = dyn_cast<FunctionDecl>(LocationDecl)) {
> if (FuncDecl->isThisDeclarationADefinition() && FuncDecl->getBody())
> EndLocation = FuncDecl->getBody()->getLocStart();
> } else if (auto TagDeclaration = dyn_cast<TagDecl>(LocationDecl)) {
> if (TagDeclaration->isThisDeclarationADefinition())
> EndLocation = TagDeclaration->getBraceRange().getBegin();
> } else if (auto NameDecl = dyn_cast<NamespaceDecl>(LocationDecl)) {
> SourceLocation BeforeLBraceLoc = Lexer::getLocForEndOfToken(
> LocationDecl->getLocation(), 0, SourceMgr, LangOpts);
> if (BeforeLBraceLoc.isValid())
> EndLocation = BeforeLBraceLoc;
> }
>
> SourceRange SR(BeginLocation, EndLocation);
> if (SR.isValid()) {
> auto L = getDeclarationLocation(AST, SR);
> if (L) {
> auto Ref = getBufferDataFromSourceRange(AST, *L);
> if (Ref) {
> Hover H;
> if (SourceMgr.isInMainFile(BeginLocation))
> H.range = L->range;
> MarkedString MS = {"", "C++", *Ref};
> HoverContents.push_back(MS);
> H.contents = std::move(HoverContents);
> return H;
> }
> }
> }
> }
>
> auto MacroInfos = DeclMacrosFinder->takeMacroInfos();
> if (!MacroInfos.empty() && MacroInfos[0]) {
> const MacroInfo *MacroInf = MacroInfos[0];
> SourceRange SR(MacroInf->getDefinitionLoc(),
> MacroInf->getDefinitionEndLoc());
> if (SR.isValid()) {
> auto L = getDeclarationLocation(AST, SR);
> if (L) {
> auto Ref = getBufferDataFromSourceRange(AST, *L);
> if (Ref) {
> Hover H;
> if (SourceMgr.isInMainFile(SR.getBegin()))
> H.range = L->range;
> MarkedString MS = {"", "C++", "#define " + Ref->str()};
> H.contents.push_back(MS);
> return H;
> }
> }
> }
> }
>
> return Hover();
> }
> ```
```
if (SourceMgr.isInMainFile(SR.getBegin()))
H.range = L->range;
```
This is not quite right in case you have a macro definition in the preamble but still in the same "document" (they are different FileIDs for the SourceManager!). For example:
```
#include "foo.h"
#define MY_MACRO 1
void bar() {
MY_MACRO;
}
```
The #define is *also* part of the preamble so isMainFile will return false. So in that case, I'm not sure what's the best way to make sure the macro expansion and the definition are in the same file but comparing FileEntry should work although a bit verbose:
```
FileID FID = SourceMgr.getFileID(SourceMgr.getExpansionLoc(SR.getBegin()));
bool IsInMainFile = FID.isValid() && SourceMgr.getMainFileID() == FID;
if (!IsInMainFile) {
// Special case when a macro is defined in a preamble, it could still be in the "main file", below the inclusions.
// The SourceManager considers the preamble and the main file as two different FileIDs but the FileEntries should match.
bool IsInPreamble = FID == SourceMgr.getPreambleFileID();
if (IsInPreamble)
IsInMainFile = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID()) == SourceMgr.getFileEntryForID(FID);
}
if (IsInMainFile)
H.range = L->range;
```
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D35894
More information about the cfe-commits
mailing list