malaperle added inline comments.
================
Comment at: clangd/ClangdUnit.cpp:1117
+
+ if (!DeclVector.empty()) {
+ const Decl *LocationDecl = DeclVector[0];
----------------
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();
}
```
================
Comment at: clangd/ClangdUnit.cpp:1172
+ Location L = getDeclarationLocation(AST, SR);
+ H.range = L.range;
+ Ref = getDataBufferFromSourceRange(AST, SR, L);
----------------
The range could be in another file but we can only highlight things in the file
that the user current has open. For example, if I'm foo.cpp and I hover on
something declared in foo.h, it will change the background color in foo.cpp but
using the line numbers of foo.h! The protocol should be more clear about this
but since there are no Uri in the Hover struct, we can assume this range should
only apply to the open file, i.e. the main file. So I suggest this check:
if (SourceMgr.isInMainFile(BeginLocation))
H.range = L->range;
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D35894
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
