[clangd] Code hover for Clangd

[Nebiroth]

Summary

Implemention of Code Hover as described in LSP definition.

[malaperle]

clangd/ClangdUnit.cpp

1077

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.

[malaperle]

clangd/ClangdUnit.cpp

1133

I think it has to do with macro expansions, the plot thickens..

[malaperle]

clangd/ClangdUnit.cpp

1133

ObjCMethodDecl is forward-declared in astfwd.h as part of a macro expansion. We have to either use the expansion loc or spelling loc to get the appropriate file entry. Probably expansion loc makes more sense but I'll test a bit.

[malaperle]

clangd/Protocol.h

453

The doc should use /// like the others

[malaperle]

clangd/ClangdUnit.cpp

1095

Not sure why but I don't get the comments when I hover on "push_back" but I do get it on many other methods. This can be investigated separately I think. It could be a bug in getRawCommentForDeclNoCache

[malaperle]

clangd/ClangdUnit.h

326

I think this can be only in the source file, in a an anonymous namespace.

[malaperle]

clangd/ClangdUnit.cpp

1034

const FileEntry *F =
        SourceMgr.getFileEntryForID(SourceMgr.getFileID(LocStart));
if (!F)
  return llvm::errc::no_such_file_or_directory;

1044

FE can be null. How about returning a llvm::ErrorOr ?

1080

FE can be null. How about returning a llvm::ErrorOr ?

[Nebiroth]

Invalid FileEntries now return llvm:ErrorOr

[malaperle]

clangd/ClangdUnit.cpp

1077

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();
}

1132

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;

[malaperle]

clangd/Protocol.cpp

1025

remove, we have to return the contents of the H that was passed as parameter, not a new one. I hit this bug while testing with no range (hover text in another file)

[malaperle]

I copy/pasted the comments again so that you don't miss them.

clangd/ClangdUnit.cpp

1077

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();
}

1132

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;

[malaperle]

clangd/ClangdUnit.cpp

1029

llvm::ErrorOr<Location>

1033

To fix the macro expansion crash:

SourceLocation LocStart = SourceMgr.getExpansionLoc(ValSourceRange.getBegin());
const FileEntry *F =
        SourceMgr.getFileEntryForID(SourceMgr.getFileID(LocStart));

and simple early return

if (!F)
  return llvm::errc::no_such_file_or_directory;

1037

getExpansionLineNumber

1038

getExpansionColumnNumber

1040

getExpansionLineNumber

1041

getExpansionColumnNumber

1045

You can do this earlier and simpler, see comment above.

[malaperle]

clangd/ClangdUnit.cpp

1077

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;

[Nebiroth]

Simplified getHover() function
Proper usage of ErrorOr to return errors
Given range for Hover struct now only applies to the open file
Fixed crash on MacroExpansion

[malaperle]

clangd/ClangdLSPServer.cpp

228

remove extra line

clangd/ClangdUnit.cpp

1039

else is not needed since you return in the if

1064

else is not needed because it returns in the if

1163

else
           H.range = Range();

Remove this, once the bug in Protocol.cpp is fixed, the Range will be truly optional.

1201

else
           H.range = Range();

Remove this, once the bug in Protocol.cpp is fixed, the Range will be truly optional.

1500

revert this change

clangd/Protocol.cpp

1025

remove, we have to return the contents of the H that was passed as parameter, not a new one. I hit this bug while testing with no range (hover text in another file)

So this should be

if (H.range.hasValue()) {
    return json::obj{
        {"contents", json::ary(H.contents)},
        {"range", H.range.getValue()},
    };
  }

  return json::obj{
    {"contents", json::ary(H.contents)},
};

clangd/Protocol.h

27

revert this change?

481

Documentation should use /// like the others

486

Documentation should use /// like the others

[Nebiroth]

clangd/Protocol.h

27

#include <string> is not needed.

[malaperle]

clangd/Protocol.h

27

I meant removing YAMLParser.h

[Nebiroth]

clangd/Protocol.h

27

That's what I figured. I'll remove that.

[Nebiroth]

Minor code cleanup
Merge with master

[malaperle]

This looks good to me. @ilya-biryukov Any objections?
I think we can do further iterations later to handle macros and other specific cases (multiple Decls at the position, etc) . It's already very useful functionality.

[malaperle]

Needs to be rebased.

[Nebiroth]

Rebase on master

[ilya-biryukov]

clangd/ClangdLSPServer.cpp

247

NIT: remove empty line

clangd/ClangdServer.cpp

442

We don't need to capture this, remove it from the capture list.

509–535

Return an error instead of asserting to follow how other functions do that:

if (!Resources)
  return llvm::make_error<llvm::StringError>(
      "findHover called on non-added file",
      llvm::errc::invalid_argument);

clangd/ClangdServer.h

293

Please put this right after findDocumentHighlights. It is really weird to see findHover in the middle of formatting functions.

296

These functions are duplicates of existing ones, they should not be here.
Bad merge?

clangd/ClangdUnit.cpp

267

NIT: remove the empty line

943

NIT: remove the empty line

956–957

Bad merge? This comment should not be changed.

958–959

std::move(Decls) is better. Bad merge?

959

This comment is not upstream now and we're removed it in the other patch. Remove it here too.

1028–1055

NIT: llvm::Expected instead of llvm::ErrorOr

1028–1055

Why are we changing the semantics here? Why should we use expansion locations instead of spelling locations here?

1032

We should avoid doing the string-matching when we have the AST.
Use something like this instead (haven't tested, but you should get the idea):

/// Returns a NamedDecl that could be used to print the qualifier.
Decl* getScopeOfDecl(Decl* D) {
  // Code from NamedDecl::printQualifiedName, we should probably move it into 
  const DeclContext *Ctx = D->getDeclContext();

  // For ObjC methods, look through categories and use the interface as context.
  if (auto *MD = dyn_cast<ObjCMethodDecl>(D))
    if (auto *ID = MD->getClassInterface())
      Ctx = ID;

  if (Ctx->isFunctionOrMethod()) {
    /// This is a function-local entity.
    return nullptr;
  }

  return Ctx;
}


std::string printScopeOfDecl(Decl* Scope) {
  if (isa<TranslationUnitDecl>(Scope))
    return "global namespace";

  if (isa<NamedDecl>(Scope))
    return cast<NamedDecl>(Scope)->printQualifiedName();

  return "";
}


// Later use like this:
auto ScopeText = printScopeOfDecl(getScopeOfDecl(D));

1056

NIT: llvm::Expected instead of llvm::ErrorOr

1074

the indent is off here. Use clang-format.

1074

Body of this if statement is a great candidate to be extracted into a separate function. Same for macros. The code is really hard to read as it is right now.

std::vector<MarkedString> getHoverContents(Decl *D);
std::vector<MarkedString> getHoverContents(MacroDef *M);
`

1089

This code seems pretty involved. Don't RawComments handle this case?

1467

This should clearly go before the comment!
Also, we're currently setting this flag when building the preamble, but not when parsing the AST. We should definitely do that in both cases.

clangd/Protocol.h

453

NIT: remove empty line

479

NIT: remove empty line

482

NIT: remove empty comment line

488

NIT: remove empty comment line

test/clangd/initialize-params-invalid.test

33

indent is off by one

test/clangd/initialize-params.test

34

indent is off by one

[Nebiroth]

clangd/ClangdServer.h

296

Yes, this is a bad merge.

clangd/ClangdUnit.cpp

1028–1055

Bad merge.

1032

I'm not sure I understand. The intention was to have a separate MarkedString display the scope for whatever we are hovering on. The way I understand the above code, this would display a scope that isn't as precise as I would like which defeats the purpose of having this feature in the first place.

1089

Bad merge.

1467

Do you mean also adding this line somewhere in ASTUnit.cpp?

[Nebiroth]

findHover properly returns an error
Removed many cases of bad merging
Separated getHover in two functions
Minor indenting and NIT fixes

[sammccall]

clangd/ClangdUnit.cpp

1467

Doesn't this slow down parsing by a lot? I thought @ilya-biryukov had seen that it was responsible for big performance losses in code complete.

ISTM we might prefer to look up the comment from the index by USR, and just live without it if we can't find it.

(Apologies if this has been discussed)

clangd/ClangdUnit.h

1

Please don't cram more stuff into clangdunit that's not directly related to building ASTs and managing their lifetimes. Instead, can we pull as much as possible out into Documentation.h or similar?

Note not Hover.h as there are other use cases, like we should use this logic for the code completion item documentation too.

test/clangd/hover.test

2

Could I ask you to do primary/exhaustive testing of this with unit tests, and just a basic smoke test with lit?
Then it's possible to read the tests, understand whether they're correct, and understand the failure messages.

I'm trying to pull more of the features in this direction - see CodeCompleteTests.cpp for an example of how this can work.

We've had quite a few cases of bugs that had tests, but nobody noticed that the tests were wrong because it's just not possible to read them.

[simark]

New version

Here's a new version of the patch, quite reworked. I scaled down the scope a
little bit to make it simpler (see commit log). We'll add more features later
on.

[ilya-biryukov]

Thanks for picking this up!
I have just one major comment related to how we generate the hover text.

Current implementation tries to get the actual source code for every declaration and then patch it up to look nice on in the output.
It is quite a large piece of code (spanning most of the patch) and tries to mix and match source code and output from the AST when there's not enough information in the AST (e.g. tryFixupIndentation and stripOpeningBracket). This approach is really complex as we'll inevitably try to parse "parts" of C++ and figure out how to deal with macros, etc.
Worse than that, I would expect this code to grow uncontrollably with time and be very hard to maintain.
Moreover, in presence of macros it arguably gives non-useful results. For example, consider this:

#define DEFINITIONS int foo(); int bar(); int baz();

// somewhere else
DEFINITIONS

// somewhere else
int test() {
  foo(); // <-- hover currently doesn't work here. And even if it did, showing a line with just DEFINITIONS is not that useful.
}

I suggest we move to a different approach of pretty-printing the relevant parts of the AST. It is already implemented in clang, handles all the cases in the AST (and will be updated along when AST is changed), shows useful information in presence of macros and is much easier to implement.
The version of getHoverContents using this is just a few lines of code:

static std::string getHoverContents(const Decl *D) {
  if (TemplateDecl *TD = D->getDescribedTemplate())
    D = TD; // we want to see the template bits.

  std::string Result;
  llvm::raw_string_ostream OS(Result);

  PrintingPolicy Policy(D->getASTContext().getLangOpts());
  Policy.TerseOutput = true;
  D->print(OS, Policy);

  OS.flush();
  return Result;
}

It doesn't add the "Defined in ..." piece, but illustrates the APIs we can use. We should use the same APIs for the scope as well, avoiding fallbacks to manual printing if we can.
If there's something missing/wrong in the upstream pretty printers, it's fine and we can fix them (e.g., the thing that I've immediately noticed is that namespaces are always printed with curly braces).

clangd/ClangdLSPServer.cpp

247

NIT: remove the empty line at the start of function

clangd/Protocol.cpp

1018

NIT: use nullptr instead of NULL
(irrelevant if we use StringRef, see other comment below)

1025

• StringRef is a nicer abstraction, maybe use it instead of const char*?
• Maybe move declaration of KindStr closer to its usage?
• Maybe extract a helper function to mark the code path for invalid kinds as unreachable? I.e.

static StringRef toTextKind(MarkupKind Kind) {
  switch (Kind) {
    case MarkupKind::PlainText:
      return "plaintext";
    case MarkupKind::Markdown:
      return "markdown";
  }
  llvm_unreachable("Invalid MarkupKind");
}

clangd/XRefs.cpp

326 ↗

(On Diff #133367)

Same suggestion as before. Could we extract a helper function to mark invalid enum values unreachable?

552 ↗

(On Diff #133367)

This assert seems rather unlikely and just makes the code more complex. Let's assume it's true and remove it (along with the subsequent if statement)

560 ↗

(On Diff #133367)

This assert seems rather unlikely and just makes the code more complex. Let's assume it's true and remove it (along with the subsequent if statement)

test/clangd/hover.test

3

We have a more readable format for the lit-tests now. Could we update this test to use it?
(see other tests in test/clangd for example)

unittests/clangd/XRefsTests.cpp

231 ↗

(On Diff #133367)

NIT: remove empty line at the start of the function

233 ↗

(On Diff #133367)

NIT: Use StringRef instead of const char*

[simark]

In D35894#1001875, @ilya-biryukov wrote:

Thanks for picking this up!
I have just one major comment related to how we generate the hover text.

Current implementation tries to get the actual source code for every declaration and then patch it up to look nice on in the output.
It is quite a large piece of code (spanning most of the patch) and tries to mix and match source code and output from the AST when there's not enough information in the AST (e.g. tryFixupIndentation and stripOpeningBracket). This approach is really complex as we'll inevitably try to parse "parts" of C++ and figure out how to deal with macros, etc.
Worse than that, I would expect this code to grow uncontrollably with time and be very hard to maintain.
Moreover, in presence of macros it arguably gives non-useful results. For example, consider this:

#define DEFINITIONS int foo(); int bar(); int baz();

// somewhere else
DEFINITIONS

// somewhere else
int test() {
  foo(); // <-- hover currently doesn't work here. And even if it did, showing a line with just DEFINITIONS is not that useful.
}

I suggest we move to a different approach of pretty-printing the relevant parts of the AST. It is already implemented in clang, handles all the cases in the AST (and will be updated along when AST is changed), shows useful information in presence of macros and is much easier to implement.
The version of getHoverContents using this is just a few lines of code:

static std::string getHoverContents(const Decl *D) {
  if (TemplateDecl *TD = D->getDescribedTemplate())
    D = TD; // we want to see the template bits.

  std::string Result;
  llvm::raw_string_ostream OS(Result);

  PrintingPolicy Policy(D->getASTContext().getLangOpts());
  Policy.TerseOutput = true;
  D->print(OS, Policy);

  OS.flush();
  return Result;
}

It doesn't add the "Defined in ..." piece, but illustrates the APIs we can use. We should use the same APIs for the scope as well, avoiding fallbacks to manual printing if we can.
If there's something missing/wrong in the upstream pretty printers, it's fine and we can fix them (e.g., the thing that I've immediately noticed is that namespaces are always printed with curly braces).

I thought it would be nice to show in the hover the declaration formatted as it is written in the source file, because that's how the developer is used to read it. But on the other hand, I completely agree that trying to do string formatting ourselves is opening a can of worms. I'll try the approach you suggest.

[simark]

clangd/ClangdLSPServer.cpp

247

Done.

clangd/Protocol.cpp

1018

Ack.

1025

Done.

clangd/XRefs.cpp

326 ↗

(On Diff #133367)

Done.

552 ↗

(On Diff #133367)

Done...

560 ↗

(On Diff #133367)

... and done.

test/clangd/hover.test

3

Indeed the new format looks much more readable. I'll modify this one to look like completion.test.

unittests/clangd/XRefsTests.cpp

231 ↗

(On Diff #133367)

Done.

233 ↗

(On Diff #133367)

Done.

[simark]

Another example where pretty-printing the AST gives better results:

int x, y = 5, z = 6;

Hover the z will now show int z = 6, before it would have shown int x, y = 5, z = 6. I think new version is better because it only shows the variable we care about.

Is there something similar we can do when printing macros, or for them the approach of getting the text from the source file is still the best thing we have?

[simark]

The only problem I see is that when hovering a function/struct name, it now prints the whole function/struct definition. When talking with @malaperle, he told me that you had discussed it before and we should not have the definition in the hover, just the prototype for a function and the name (e.g. "struct foo") for a struct. Glancing quickly at the PrintingPolicy, I don't see a way to do that. Do you have any idea?

[simark]

In D35894#1003342, @simark wrote:

The only problem I see is that when hovering a function/struct name, it now prints the whole function/struct definition. When talking with @malaperle, he told me that you had discussed it before and we should not have the definition in the hover, just the prototype for a function and the name (e.g. "struct foo") for a struct. Glancing quickly at the PrintingPolicy, I don't see a way to do that. Do you have any idea?

Arrg, Sorry, I just re-read your comment and saw that you used TerseOutput which does that. I did not catch that when transcribing the code (I did not want to copy paste to understand the code better, but got bitten).

[ilya-biryukov]

In D35894#1003356, @simark wrote:

In D35894#1003342, @simark wrote:

The only problem I see is that when hovering a function/struct name, it now prints the whole function/struct definition. When talking with @malaperle, he told me that you had discussed it before and we should not have the definition in the hover, just the prototype for a function and the name (e.g. "struct foo") for a struct. Glancing quickly at the PrintingPolicy, I don't see a way to do that. Do you have any idea?

Arrg, Sorry, I just re-read your comment and saw that you used TerseOutput which does that. I did not catch that when transcribing the code (I did not want to copy paste to understand the code better, but got bitten).

Yeah, the TerseOutput bit was important :-)

In D35894#1003342, @simark wrote:

The only problem I see is that when hovering a function/struct name, it now prints the whole function/struct definition. When talking with @malaperle, he told me that you had discussed it before and we should not have the definition in the hover, just the prototype for a function and the name (e.g. "struct foo") for a struct. Glancing quickly at the PrintingPolicy, I don't see a way to do that. Do you have any idea?

I'm not aware of the code that pretty-prints macros. There's a code that dumps debug output, but it's not gonna help in that case.
Alternatively, we could simply print the macro name and not print the definition. That's super-simple and is in line with hovers for the AST decls. Let's do that in the initial version.

For macros using the source code is also less of a problem, so we may try to do it. C++ doesn't allow to generate a macro inside another macro, so we don't run into the same problems we may hit with the AST. And the overall syntax is much simpler than for AST, so we can hope to get string manipulation right with reasonable effort. (Simply reindenting should do the trick).

[simark]

I'm not aware of the code that pretty-prints macros. There's a code that dumps debug output, but it's not gonna help in that case.
Alternatively, we could simply print the macro name and not print the definition. That's super-simple and is in line with hovers for the AST decls. Let's do that in the initial version.

Ok, I'm considering printing "#define MACRO". Just printing the name would not be very useful, at least printing "#define" gives the information that the hovered entity is defined as a macro (and not a function or variable).

Is there a way to get the macro name from the MacroInfo object? I couldn't find any, so the solution I'm considering is to make DeclarationAndMacrosFinder::takeMacroInfos return an std::vector<std::pair<StringRef, const MacroInfo *>>, where the first member of the pair is the macro name. It would come from IdentifierInfo->getName() in DeclarationAndMacrosFinder::finish. Does that make sense, or is there a simpler way?

[ilya-biryukov]

In D35894#1006124, @simark wrote:

Is there a way to get the macro name from the MacroInfo object? I couldn't find any, so the solution I'm considering is to make DeclarationAndMacrosFinder::takeMacroInfos return an std::vector<std::pair<StringRef, const MacroInfo *>>, where the first member of the pair is the macro name. It would come from IdentifierInfo->getName() in DeclarationAndMacrosFinder::finish. Does that make sense, or is there a simpler way?

I don't think there's a way to get macro name from MacroInfo. pair<Name, MacroInfo*> sounds good to me, I'd probably even use a named struct here: struct MacroDecl { StringRef Name; const MacroInfo &Info; }

[simark]

Generate Hover by pretty-printing the AST

This new version of the patch generates the hover by pretty-printing the AST.
The unit tests are updated accordingly. There are some inconsistencies in how
things are printed. For example, I find it a bit annoying that we print empty
curly braces after class names (e.g. "class Foo {}"). Also, namespace and
enums are printed with a newline between the two braces. These are things that
could get fixed by doing changes to the clang AST printer.

The hover.test test now uses a more readable format.

[simark]

unittests/clangd/XRefsTests.cpp

561 ↗

(On Diff #134306)

Note that I used .str() here to make the output of failing tests readable and useful. By default, gtest tries to print StringRef as if it was a container. I tried to make add a PrintTo function to specify how it should be printed, but couldn't get it to work (to have it called by the compiler), so I settled for this.

[ilya-biryukov]

Just a few last remarks and this is good to go.
Should I land it for you after the last comments are fixed?

clangd/XRefs.cpp

354 ↗

(On Diff #134306)

We should call flush() before returning Name here. The raw_string_ostream is buffered.

373 ↗

(On Diff #134306)

NIT: use llvm::None here instead of {}

394 ↗

(On Diff #134306)

Accidental leftover from previous code?

unittests/clangd/XRefsTests.cpp

262 ↗

(On Diff #134306)

NIT: LLVM uses UpperCamelCase for field names.

561 ↗

(On Diff #134306)

Thanks for spotting that.
We have a fix for that in LLVM's gtest extensions (D43330).
str() can now be removed.

[simark]

clangd/XRefs.cpp

354 ↗

(On Diff #134306)

I replaced it with Stream.str().

373 ↗

(On Diff #134306)

Done.

394 ↗

(On Diff #134306)

Oops, thanks.

unittests/clangd/XRefsTests.cpp

262 ↗

(On Diff #134306)

Ok, I fixed all the structs this patch adds.

561 ↗

(On Diff #134306)

Removed. Even if this patch is merged before D43330 it's not a big deal, since it only affects debug output of failing test cases.

[simark]

New version

• Rebase on master, change findHover to have a callback-based interface
• Fix nits from previous review comments

[simark]

In D35894#1008593, @ilya-biryukov wrote:

Just a few last remarks and this is good to go.
Should I land it for you after the last comments are fixed?

I just received my commit access, so if you are ok with it, I would try to push it once I get your final OK.

[ilya-biryukov]

Only the naming of fields left.

Also, please make sure to run git-clang-format on the code before submitting to make sure it's formatted properly.

clangd/ClangdLSPServer.cpp

250

NIT: use return replyError to be consistent with other methods.

clangd/XRefs.cpp

354 ↗

(On Diff #134306)

Also works, albeit less efficient (will probably do a copy where move is enough).

unittests/clangd/XRefsTests.cpp

262 ↗

(On Diff #134306)

Argh, sorry that I didn't mention it before, but we have an exception for structs in Procotol.h, they follow the spelling in the LSP spec (i.e. should be lowerCamelCase).

[simark]

In D35894#1008861, @ilya-biryukov wrote:

Only the naming of fields left.

Also, please make sure to run git-clang-format on the code before submitting to make sure it's formatted properly.

Ahh, I was running clang-format by hand, didn't know about git-clang-format (and obviously forgot some files). Thanks.

clangd/ClangdLSPServer.cpp

250

Ok.

clangd/XRefs.cpp

354 ↗

(On Diff #134306)

Ah, didn't think about that. I'll change it to flush + return Name.

unittests/clangd/XRefsTests.cpp

262 ↗

(On Diff #134306)

Haha, no problem, changing them back is one keystroke away.

[simark]

Fix some nits

• Revert case change of struct fields in Protocol.h
• Change return formatting in onHover
• Use flush() + return Name in NamedDeclQualifiedName and TypeDeclToString

[ilya-biryukov]

Thanks for fixing all the NITs!

[simark]

Fix field names in XRefsTests.cpp

I realized I forgot to add some fixups in the previous version, the field names
in XRefsTests.cpp were not updated and therefore it did not build.

[ilya-biryukov]

LGTM

[simark]

Rebase

[simark]

Rebase

[malaperle]

Rebase.

[malaperle]

Add a missing std::move, fix some formatting.

[malaperle]

Works well and code looks good. There were only minor tweaks since the last approval. I'll land this since there are some issues with Simon's svn account.

clangd/ClangdLSPServer.h

private:
   void onCodeAction(Ctx C, CodeActionParams &Params) override;
   void onCompletion(Ctx C, TextDocumentPositionParams &Params) override;
   void onSignatureHelp(Ctx C, TextDocumentPositionParams &Params) override;
   void onGoToDefinition(Ctx C, TextDocumentPositionParams &Params) override;
   void onSwitchSourceHeader(Ctx C, TextDocumentIdentifier &Params) override;
   void onFileEvent(Ctx C, DidChangeWatchedFilesParams &Params) override;
   void onCommand(Ctx C, ExecuteCommandParams &Params) override;
   void onRename(Ctx C, RenameParams &Parames) override;
+  void onCodeHover(Ctx C, TextDocumentPositionParams &Params) override;
 
   std::vector<clang::tooling::Replacement>
   getFixIts(StringRef File, const clangd::Diagnostic &D);
 
   JSONOutput &Out;
   /// Used to indicate that the 'shutdown' request was received from the
   /// Language Server client.
   bool ShutdownRequestReceived = false;

clangd/ClangdLSPServer.cpp

 //===--- ClangdLSPServer.cpp - LSP server ------------------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===---------------------------------------------------------------------===//
 
 #include "ClangdLSPServer.h"
 #include "JSONRPCDispatcher.h"
 
 #include "llvm/Support/FormatVariadic.h"
 
 using namespace clang::clangd;
 using namespace clang;
 
 namespace {
 
C.reply(json::obj{
                  {"resolveProvider", false},
                  {"triggerCharacters", {".", ">", ":"}},
              }},
             {"signatureHelpProvider",
              json::obj{
                  {"triggerCharacters", {"(", ","}},
              }},
             {"definitionProvider", true},
+            {"hoverProvider", true},
             {"renameProvider", true},
             {"executeCommandProvider",
              json::obj{
                  {"commands", {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND}},
              }},
         }}}});
   if (Params.rootUri && !Params.rootUri->file.empty())
     Server.setRootPath(Params.rootUri->file);
auto List = Server
                   .Value;
   C.reply(List);
 }
 
 void ClangdLSPServer::onSignatureHelp(Ctx C,
                                       TextDocumentPositionParams &Params) {
   auto SignatureHelp = Server.signatureHelp(
       Params.textDocument.uri.file,
       Position{Params.position.line, Params.position.character});
   if (!SignatureHelp)
     return C.replyError(ErrorCode::InvalidParams,
                         llvm::toString(SignatureHelp.takeError()));
   C.reply(SignatureHelp->Value);
 }
 
 void ClangdLSPServer::onGoToDefinition(Ctx C,
                                        TextDocumentPositionParams &Params) {
   auto Items = Server.findDefinitions(
       Params.textDocument.uri.file,
       Position{Params.position.line, Params.position.character});
   if (!Items)
     return C.replyError(ErrorCode::InvalidParams,
                         llvm::toString(Items.takeError()));
-  C.reply(json::ary(Items->Value));
+
+  std::vector<Location> LocationVector;
+  auto test = Items->Value;
+
+  for (auto Item : Items->Value) {
+    LocationVector.push_back(Item);
+  }
+
+  C.reply(json::ary(LocationVector));
 }
 
 void ClangdLSPServer::onSwitchSourceHeader(Ctx C,
                                            TextDocumentIdentifier &Params) {
   llvm::Optional<Path> Result = Server.switchSourceHeader(Params.uri.file);
   std::string ResultUri;
   C.reply(Result ? URI::fromFile(*Result).uri : "");
 }
 
+void ClangdLSPServer::onCodeHover(Ctx C, TextDocumentPositionParams &Params) {
+
+  auto H = Server.findHover(
+      Params.textDocument.uri.file,
+      Position{Params.position.line, Params.position.character});
+  if (!H)
+    return C.replyError(ErrorCode::InvalidParams,
+                        llvm::toString(H.takeError()));
+
+  C.reply(Hover::unparse(H->Value));
+}
+
 ClangdLSPServer::ClangdLSPServer(JSONOutput &Out, unsigned AsyncThreadsCount,
                                  bool StorePreamblesInMemory,
                                  bool SnippetCompletions,
                                  llvm::Optional<StringRef> ResourceDir,
                                  llvm::Optional<Path> CompileCommandsDir)
     : Out(Out), CDB(/*Logger=*/Out, std::move(CompileCommandsDir)),
       Server(CDB, /*DiagConsumer=*/*this, FSProvider, AsyncThreadsCount,
              StorePreamblesInMemory,
              clangd::CodeCompleteOptions(
                  /*EnableSnippetsAndCodePatterns=*/SnippetCompletions),

clangd/ClangdServer.h

public:
 
   template <class U>
   Tagged(const Tagged<U> &Other) : Value(Other.Value), Tag(Other.Tag) {}
 
   template <class U>
   Tagged(Tagged<U> &&Other)
       : Value(std::move(Other.Value)), Tag(std::move(Other.Tag)) {}
 
   T Value = T();
   VFSTag Tag = VFSTag();
 };
 
 template <class T>
 Tagged<typename std::decay<T>::type> make_tagged(T &&Value, VFSTag Tag) {
   return Tagged<typename std::decay<T>::type>(std::forward<T>(Value), Tag);
 }
 
public:
   /// vfs::FileSystem used for signature help. This method should only be called
   /// for currently tracked files.
   llvm::Expected<Tagged<SignatureHelp>>
   signatureHelp(PathRef File, Position Pos,
                 llvm::Optional<StringRef> OverridenContents = llvm::None,
                 IntrusiveRefCntPtr<vfs::FileSystem> *UsedFS = nullptr);
 
   /// Get definition of symbol at a specified \p Line and \p Column in \p File.
   llvm::Expected<Tagged<std::vector<Location>>> findDefinitions(PathRef File,
                                                                 Position Pos);
 
   /// Helper function that returns a path to the corresponding source file when
   /// given a header file and vice versa.
   llvm::Optional<Path> switchSourceHeader(PathRef Path);
 
+  llvm::Expected<Tagged<Hover>> findHover(PathRef File, Position Pos);
+
   /// Run formatting for \p Rng inside \p File.
   std::vector<tooling::Replacement> formatRange(PathRef File, Range Rng);
   /// Run formatting for the whole \p File.
   std::vector<tooling::Replacement> formatFile(PathRef File);
   /// Run formatting after a character was typed at \p Pos in \p File.
   std::vector<tooling::Replacement> formatOnType(PathRef File, Position Pos);
   /// Rename all occurrences of the symbol at the \p Pos in \p File to
   /// \p NewName.
   Expected<std::vector<tooling::Replacement>> rename(PathRef File, Position Pos,
                                                      llvm::StringRef NewName);
 
   /// Gets current document contents for \p File. \p File must point to a
   /// currently tracked file.
   /// FIXME(ibiryukov): This function is here to allow offset-to-Position
   /// conversions in outside code, maybe there's a way to get rid of it.
   std::string getDocument(PathRef File);
 
   /// Only for testing purposes.
   /// Waits until all requests to worker thread are finished and dumps AST for
   /// \p File. \p File must be in the list of added documents.
   std::string dumpAST(PathRef File);

clangd/ClangdServer.cpp

 //===--- ClangdServer.cpp - Main clangd server code --------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===-------------------------------------------------------------------===//
 
 #include "ClangdServer.h"
+#include "Protocol.h"
 #include "clang/Format/Format.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Refactoring/RefactoringResultConsumer.h"
 #include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FormatProviders.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include <future>
 
 using namespace clang;
 using namespace clang::clangd;
 
 namespace {
 
 class FulfillPromiseGuard {
 public:
   FulfillPromiseGuard(std::promise<void> &Promise) : Promise(Promise) {}
 
   ~FulfillPromiseGuard() { Promise.set_value(); }
 
 private:
Resources->getAST().get()->runUnderLock([&Result](ParsedAST *AST) {
     }
     ResultOS.flush();
   });
   return Result;
 }
 
 llvm::Expected<Tagged<std::vector<Location>>>
 ClangdServer::findDefinitions(PathRef File, Position Pos) {
+
   auto TaggedFS = FSProvider.getTaggedFileSystem(File);
 
   std::shared_ptr<CppFile> Resources = Units.getFile(File);
   if (!Resources)
     return llvm::make_error<llvm::StringError>(
         "findDefinitions called on non-added file",
         llvm::errc::invalid_argument);
 
   std::vector<Location> Result;
   Resources->getAST().get()->runUnderLock([Pos, &Result, this](ParsedAST *AST) {
     if (!AST)
       return;
     Result = clangd::findDefinitions(*AST, Pos, Logger);
   });
+
   return make_tagged(std::move(Result), TaggedFS.Tag);
 }
 
 llvm::Optional<Path> ClangdServer::switchSourceHeader(PathRef Path) {
 
   StringRef SourceExtensions[] = {".cpp", ".c", ".cc", ".cxx",
                                   ".c++", ".m", ".mm"};
   StringRef HeaderExtensions[] = {".h", ".hh", ".hpp", ".hxx", ".inc"};
 
   StringRef PathExt = llvm::sys::path::extension(Path);
 
   // Lookup in a list of known extensions.
   auto SourceIter =
       std::find_if(std::begin(SourceExtensions), std::end(SourceExtensions),
                    [&PathExt](PathRef SourceExt) {
                      return SourceExt.equals_lower(PathExt);
                    });
   bool IsSource = SourceIter != std::end(SourceExtensions);
for (StringRef NewExt : NewExts) {
     // Also check NewExt in upper-case, just in case.
     llvm::sys::path::replace_extension(NewPath, NewExt.upper());
     if (FS->exists(NewPath))
       return NewPath.str().str();
   }
 
   return llvm::None;
 }
 
+llvm::Expected<Tagged<Hover>> ClangdServer::findHover(PathRef File,
+                                                      Position Pos) {
+  // Default Hover values
+  std::vector<MarkedString> EmptyVector;
+  Position BeginPosition = {0, 0};
+  Position EndPosition = {0, 0};
+  Range DefaultRange = {BeginPosition, EndPosition};
+  Hover FinalHover = {EmptyVector, DefaultRange};
+
+  auto TaggedFS = FSProvider.getTaggedFileSystem(File);
+
+  std::shared_ptr<CppFile> Resources = Units.getFile(File);
+  assert(Resources && "Calling findHover on non-added file");
+  Resources->getAST().get()->runUnderLock(
+      [Pos, &FinalHover, this](ParsedAST *AST) {
+        if (!AST)
+          return;
+        // Only show hover for the first found definition, we could potentially
+        // expand this in a later patch.
+        FinalHover = clangd::getHover(*AST, Pos, Logger);
+      });
+
+  return make_tagged(std::move(FinalHover), TaggedFS.Tag);
+}
+
 std::future<void> ClangdServer::scheduleReparseAndDiags(
     PathRef File, VersionedDraft Contents, std::shared_ptr<CppFile> Resources,
     Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS) {
 
   assert(Contents.Draft && "Draft must have contents");
   UniqueFunction<llvm::Optional<std::vector<DiagWithFixIts>>()>
       DeferredRebuild =
           Resources->deferRebuild(*Contents.Draft, TaggedFS.Value);
   std::promise<void> DonePromise;

clangd/ClangdUnit.h

 //===--- ClangdUnit.h -------------------------------------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===---------------------------------------------------------------------===//
 
public:
   /// We use std::shared_ptr here because MVSC fails to compile non-copyable
   /// classes as template arguments of promise/future. It is guaranteed to
   /// always be non-null.
   std::shared_future<std::shared_ptr<ParsedASTWrapper>> getAST() const;
 
   /// Get CompileCommand used to build this CppFile.
   tooling::CompileCommand const &getCompileCommand() const;
 
 private:
   /// A helper guard that manages the state of CppFile during rebuild.
   class RebuildGuard {
   public:
     RebuildGuard(CppFile &File, unsigned RequestRebuildCounter);
     ~RebuildGuard();
 
     bool wasCancelledBeforeConstruction() const;
 
   private:
     CppFile &File;
     unsigned RequestRebuildCounter;
     bool WasCancelledBeforeConstruction;
   };
 
   Path FileName;
   tooling::CompileCommand Command;
   bool StorePreamblesInMemory;
 
   /// Mutex protects all fields, declared below it, FileName and Command are not
   /// mutated.
   mutable std::mutex Mutex;
   /// A counter to cancel old rebuilds.
   unsigned RebuildCounter;
private:
   /// Utility class, required by clang.
   std::shared_ptr<PCHContainerOperations> PCHs;
   /// Used for logging various messages.
   clangd::Logger &Logger;
 };
 
 struct CodeCompleteOptions {
   CodeCompleteOptions() = default;
 
   /// Uses default values for all flags, but sets EnableSnippets and
   /// IncludeCodePatterns to the value of EnableSnippetsAndCodePatterns.
   explicit CodeCompleteOptions(bool EnableSnippetsAndCodePatterns);
 
   CodeCompleteOptions(bool EnableSnippets, bool IncludeCodePatterns,
                       bool IncludeMacros, bool IncludeGlobals,
                       bool IncludeBriefComments);
 

 /// Get the beginning SourceLocation at a specified \p Pos.
 SourceLocation getBeginningOfIdentifier(ParsedAST &Unit, const Position &Pos,
                                         const FileEntry *FE);
 
 /// Get definition of symbol at a specified \p Pos.
 std::vector<Location> findDefinitions(ParsedAST &AST, Position Pos,
                                       clangd::Logger &Logger);
 
+Hover getHover(ParsedAST &AST, Position Pos, clangd::Logger &Logger);
+
 /// For testing/debugging purposes. Note that this method deserializes all
 /// unserialized Decls, so use with care.
 void dumpAST(ParsedAST &AST, llvm::raw_ostream &OS);
 
 } // namespace clangd
 } // namespace clang
 #endif

clangd/ClangdUnit.cpp

 
 #include "Logger.h"
 #include "Trace.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Frontend/Utils.h"
 #include "clang/Index/IndexDataConsumer.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Serialization/ASTWriter.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/ArrayRef.h"
createCompilerInvocation(ArrayRef<const char *> ArgList,
                          IntrusiveRefCntPtr<DiagnosticsEngine> Diags,
                          IntrusiveRefCntPtr<vfs::FileSystem> VFS) {
   auto CI = createInvocationFromCommandLine(ArgList, std::move(Diags),
                                             std::move(VFS));
   // We rely on CompilerInstance to manage the resource (i.e. free them on
   // EndSourceFile), but that won't happen if DisableFree is set to true.
   // Since createInvocationFromCommandLine sets it to true, we have to override
   // it.
+  CI->LangOpts->CommentOpts.ParseAllComments = true;
   CI->getFrontendOpts().DisableFree = false;
   return CI;
 }
 
 /// Creates a CompilerInstance from \p CI, with main buffer overriden to \p
 /// Buffer and arguments to read the PCH from \p Preamble, if \p Preamble is not
 /// null. Note that vfs::FileSystem inside returned instance may differ from \p
 /// VFS if additional file remapping were set in command-line arguments.
prepareCompilerInstance(std::unique_ptr<clang::CompilerInvocation> CI,
   // RemappedFileBuffers will handle the lifetime of the Buffer pointer,
   // release it.
   Buffer.release();
   return Clang;
 }
 
 template <class T> bool futureIsReady(std::shared_future<T> const &Future) {
   return Future.wait_for(std::chrono::seconds(0)) == std::future_status::ready;
 }
 
 } // namespace
 
 namespace {
 
 CompletionItemKind getKindOfDecl(CXCursorKind CursorKind) {
   switch (CursorKind) {
   case CXCursor_MacroInstantiation:

 
 SourceLocation getMacroArgExpandedLocation(const SourceManager &Mgr,
                                            const FileEntry *FE, Position Pos) {
   SourceLocation InputLoc =
       Mgr.translateFileLineCol(FE, Pos.line + 1, Pos.character + 1);
   return Mgr.getMacroArgExpandedLocation(InputLoc);
 }
 
 /// Finds declarations locations that a given source location refers to.
 class DeclarationLocationsFinder : public index::IndexDataConsumer {
-  std::vector<Location> DeclarationLocations;
+  std::vector<const Decl *> DeclarationDecls;
+  std::vector<const MacroInfo *> DeclarationMacroInfs;
   const SourceLocation &SearchedLocation;
   const ASTContext &AST;
   Preprocessor &PP;
 
 public:
   DeclarationLocationsFinder(raw_ostream &OS,
                              const SourceLocation &SearchedLocation,
                              ASTContext &AST, Preprocessor &PP)
       : SearchedLocation(SearchedLocation), AST(AST), PP(PP) {}
 
-  std::vector<Location> takeLocations() {
+  std::vector<const Decl *> takeDecls() {
     // Don't keep the same location multiple times.
     // This can happen when nodes in the AST are visited twice.
-    std::sort(DeclarationLocations.begin(), DeclarationLocations.end());
-    auto last =
-        std::unique(DeclarationLocations.begin(), DeclarationLocations.end());
-    DeclarationLocations.erase(last, DeclarationLocations.end());
-    return std::move(DeclarationLocations);
+    std::sort(DeclarationDecls.begin(), DeclarationDecls.end());
+    auto Last = std::unique(DeclarationDecls.begin(), DeclarationDecls.end());
+    DeclarationDecls.erase(Last, DeclarationDecls.end());
+    return std::move(DeclarationDecls);
+  }
+
+  std::vector<const MacroInfo *> takeMacroInfos() {
+    // Don't keep the same Macro info multiple times.
+    // This can happen when nodes in the AST are visited twice.
+    std::sort(DeclarationMacroInfs.begin(), DeclarationMacroInfs.end());
+    auto Last =
+        std::unique(DeclarationMacroInfs.begin(), DeclarationMacroInfs.end());
+    DeclarationMacroInfs.erase(Last, DeclarationMacroInfs.end());
+    return std::move(DeclarationMacroInfs);
   }
 
   bool
   handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
                       ArrayRef<index::SymbolRelation> Relations, FileID FID,
                       unsigned Offset,
                       index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
-    if (isSearchedLocation(FID, Offset)) {
-      addDeclarationLocation(D->getSourceRange());
-    }
+
+    if (isSearchedLocation(FID, Offset))
+      DeclarationDecls.push_back(D);
     return true;
   }
 
+  std::vector<const Decl *> getDeclarationDecls() { return DeclarationDecls; };
+  std::vector<const MacroInfo *> getMacroInfos() {
+    return DeclarationMacroInfs;
+  };
+
 private:
   bool isSearchedLocation(FileID FID, unsigned Offset) const {
     const SourceManager &SourceMgr = AST.getSourceManager();
     return SourceMgr.getFileOffset(SearchedLocation) == Offset &&
            SourceMgr.getFileID(SearchedLocation) == FID;
   }
 
-  void addDeclarationLocation(const SourceRange &ValSourceRange) {
-    const SourceManager &SourceMgr = AST.getSourceManager();
-    const LangOptions &LangOpts = AST.getLangOpts();
-    SourceLocation LocStart = ValSourceRange.getBegin();
-    SourceLocation LocEnd = Lexer::getLocForEndOfToken(ValSourceRange.getEnd(),
-                                                       0, SourceMgr, LangOpts);
-    Position Begin;
-    Begin.line = SourceMgr.getSpellingLineNumber(LocStart) - 1;
-    Begin.character = SourceMgr.getSpellingColumnNumber(LocStart) - 1;
-    Position End;
-    End.line = SourceMgr.getSpellingLineNumber(LocEnd) - 1;
-    End.character = SourceMgr.getSpellingColumnNumber(LocEnd) - 1;
-    Range R = {Begin, End};
-    Location L;
-    if (const FileEntry *F =
-            SourceMgr.getFileEntryForID(SourceMgr.getFileID(LocStart))) {
-      StringRef FilePath = F->tryGetRealPathName();
-      if (FilePath.empty())
-        FilePath = F->getName();
-      L.uri = URI::fromFile(FilePath);
-      L.range = R;
-      DeclarationLocations.push_back(L);
-    }
-  }
-
   void finish() override {
     // Also handle possible macro at the searched location.
     Token Result;
     if (!Lexer::getRawToken(SearchedLocation, Result, AST.getSourceManager(),
                             AST.getLangOpts(), false)) {
       if (Result.is(tok::raw_identifier)) {
         PP.LookUpIdentifierInfo(Result);
       }
       IdentifierInfo *IdentifierInfo = Result.getIdentifierInfo();
       if (IdentifierInfo && IdentifierInfo->hadMacroDefinition()) {
         std::pair<FileID, unsigned int> DecLoc =
             AST.getSourceManager().getDecomposedExpansionLoc(SearchedLocation);
         // Get the definition just before the searched location so that a macro
         // referenced in a '#undef MACRO' can still be found.
         SourceLocation BeforeSearchedLocation = getMacroArgExpandedLocation(
             AST.getSourceManager(),
             AST.getSourceManager().getFileEntryForID(DecLoc.first),
             DecLoc.second - 1);
         MacroDefinition MacroDef =
             PP.getMacroDefinitionAtLoc(IdentifierInfo, BeforeSearchedLocation);
         MacroInfo *MacroInf = MacroDef.getMacroInfo();
         if (MacroInf) {
-          addDeclarationLocation(SourceRange(MacroInf->getDefinitionLoc(),
-                                             MacroInf->getDefinitionEndLoc()));
+          DeclarationMacroInfs.push_back(MacroInf);
         }
       }
     }
   }
 };
 
 } // namespace
 
+Location getDeclarationLocation(ParsedAST &AST,
+                                const SourceRange &ValSourceRange) {
+  const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
+  const LangOptions &LangOpts = AST.getASTContext().getLangOpts();
+  SourceLocation LocStart = ValSourceRange.getBegin();
+  SourceLocation LocEnd = Lexer::getLocForEndOfToken(ValSourceRange.getEnd(), 0,
+                                                     SourceMgr, LangOpts);
+  Position Begin;
+  Begin.line = SourceMgr.getSpellingLineNumber(LocStart) - 1;
+  Begin.character = SourceMgr.getSpellingColumnNumber(LocStart) - 1;
+  Position End;
+  End.line = SourceMgr.getSpellingLineNumber(LocEnd) - 1;
+  End.character = SourceMgr.getSpellingColumnNumber(LocEnd) - 1;
+  Range R = {Begin, End};
+  Location L;
+  if (const FileEntry *F =
+          SourceMgr.getFileEntryForID(SourceMgr.getFileID(LocStart))) {
+    StringRef FilePath = F->tryGetRealPathName();
+    if (FilePath.empty())
+      FilePath = F->getName();
+    L.uri = URI::fromFile(FilePath);
+    L.range = R;
+  }
+  return L;
+}
+
 std::vector<Location> clangd::findDefinitions(ParsedAST &AST, Position Pos,
                                               clangd::Logger &Logger) {
   const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
   const FileEntry *FE = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
   if (!FE)
     return {};
 
   SourceLocation SourceLocationBeg = getBeginningOfIdentifier(AST, Pos, FE);
 
   auto DeclLocationsFinder = std::make_shared<DeclarationLocationsFinder>(
       llvm::errs(), SourceLocationBeg, AST.getASTContext(),
       AST.getPreprocessor());
   index::IndexingOptions IndexOpts;
   IndexOpts.SystemSymbolFilter =
       index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
 
   indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
                      DeclLocationsFinder, IndexOpts);
 
-  return DeclLocationsFinder->takeLocations();
+  std::vector<const Decl *> Decls = DeclLocationsFinder->takeDecls();
+  std::vector<const MacroInfo *> MacroInfos =
+      DeclLocationsFinder->takeMacroInfos();
+  std::vector<Location> Result;
+
+  for (auto Item : Decls) {
+    Location L = getDeclarationLocation(AST, Item->getSourceRange());
+    Result.push_back(L);
+  }
+
+  for (auto Item : MacroInfos) {
+    SourceRange SR =
+        SourceRange(Item->getDefinitionLoc(), Item->getDefinitionEndLoc());
+    Location L = getDeclarationLocation(AST, SR);
+    Result.push_back(L);
+  }
+
+  return Result;
+}
+
+Hover clangd::getHover(ParsedAST &AST, Position Pos, clangd::Logger &Logger) {
+
+  // Default Hover values
+  std::vector<MarkedString> EmptyVector;
+  Position BeginPosition = {0, 0};
+  Position EndPosition = {0, 0};
+  Range DefaultRange = {BeginPosition, EndPosition};
+  Hover H = {EmptyVector, DefaultRange};
+  unsigned Start, End;
+  StringRef Ref;
+
+  const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
+  const LangOptions &LangOpts = AST.getASTContext().getLangOpts();
+  const FileEntry *FE = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
+  if (!FE)
+    return H;
+
+  SourceLocation SourceLocationBeg = getBeginningOfIdentifier(AST, Pos, FE);
+
+  auto DeclLocationsFinder = std::make_shared<DeclarationLocationsFinder>(
+      llvm::errs(), SourceLocationBeg, AST.getASTContext(),
+      AST.getPreprocessor());
+  index::IndexingOptions IndexOpts;
+  IndexOpts.SystemSymbolFilter =
+      index::IndexingOptions::SystemSymbolFilterKind::All;
+  IndexOpts.IndexFunctionLocals = true;
+
+  indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
+                     DeclLocationsFinder, IndexOpts);
+
+  if (!DeclLocationsFinder->getDeclarationDecls().empty()) {
+    const Decl *LocationDecl = DeclLocationsFinder->takeDecls()[0];
+
+    if (LocationDecl) {
+      SourceRange SR = LocationDecl->getSourceRange();
+      if (auto FuncDecl = dyn_cast<FunctionDecl>(LocationDecl)) {
+        SourceLocation NewLocation;
+        if (AST.getASTContext().getRawCommentForDeclNoCache(LocationDecl)) {
+          NewLocation = AST.getASTContext()
+                            .getRawCommentForDeclNoCache(LocationDecl)
+                            ->getLocStart();
+          SR =
+              SourceRange(NewLocation, LocationDecl->getSourceRange().getEnd());
+        }
+        if (FuncDecl->isThisDeclarationADefinition() && FuncDecl->getBody())
+          SR = SourceRange(LocationDecl->getSourceRange().getBegin(),
+                           FuncDecl->getBody()->getLocStart());
+      } else if (auto ClassDecl = dyn_cast<TagDecl>(LocationDecl)) {
+        if (!ClassDecl->isInterface() && !ClassDecl->isUnion() &&
+            ClassDecl->isThisDeclarationADefinition())
+          SR = SourceRange(LocationDecl->getSourceRange().getBegin(),
+                           ClassDecl->getBraceRange().getBegin());
+
+      } else if (auto NameDecl = dyn_cast<NamespaceDecl>(LocationDecl)) {
+        SourceLocation BeforeLBraceLoc = Lexer::getLocForEndOfToken(
+            LocationDecl->getLocation(), 0, SourceMgr, LangOpts);
+        if (BeforeLBraceLoc.isValid())
+          SR = SourceRange(NameDecl->getLocStart(), BeforeLBraceLoc);
+
+      } else if (dyn_cast<TypedefDecl>(LocationDecl)) {
+        // FIXME: find a way to get the hover for the type that is being aliased
+        SR = LocationDecl->getSourceRange();
+      }
+      // for everything else in ValueDecl, so lvalues of variables, function
+      // designations and enum constants
+      else if (dyn_cast<ValueDecl>(LocationDecl)) {
+        SR = LocationDecl->getSourceRange();
+      }
+
+      if (SR.isValid()) {
+        Location L = getDeclarationLocation(AST, SR);
+        H.range = L.range;
+        const FileEntry *FE = SourceMgr.getFileManager().getFile(L.uri.file);
+        FileID FID = SourceMgr.translateFile(FE);
+        Ref = SourceMgr.getBufferData(FID);
+        Start = SourceMgr.getFileOffset(SourceMgr.translateFileLineCol(
+            FE, L.range.start.line + 1, L.range.start.character + 1));
+        End = SourceMgr.getFileOffset(SourceMgr.translateFileLineCol(
+            FE, L.range.end.line + 1, L.range.end.character + 1));
+        Ref = Ref.slice(Start, End);
+      }
+
+      if (const NamedDecl *NamedD = dyn_cast<NamedDecl>(LocationDecl)) {
+
+        // Get the full qualified name, the non-qualified name and then diff
+        // them. If there's something left, use that as the scope in the hover,
+        // trimming the extra "::"
+        PrintingPolicy WithScopePP(AST.getASTContext().getLangOpts());
+        std::string WithScopeBuf;
+        llvm::raw_string_ostream WithScopeOS(WithScopeBuf);
+        NamedD->printQualifiedName(WithScopeOS, WithScopePP);
+
+        // Get all contexts for current NamedDecl
+        const DeclContext *Ctx = NamedD->getDeclContext();
+
+        // For ObjC methods, look through categories and use the interface as
+        // context.
+        if (auto *MD = dyn_cast<ObjCMethodDecl>(NamedD))
+          if (auto *ID = MD->getClassInterface())
+            Ctx = ID;
+
+        typedef SmallVector<const DeclContext *, 8> ContextsTy;
+        ContextsTy Contexts;
+
+        // Collect contexts.
+        while (Ctx && isa<NamedDecl>(Ctx)) {
+          Contexts.push_back(Ctx);
+          Ctx = Ctx->getParent();
+        }
+
+        std::string ResWithScope = WithScopeOS.str();
+        if (!ResWithScope.empty()) {
+          // Get non-qualified name
+          std::string PrintedNameBuf;
+          llvm::raw_string_ostream PrintedNameOS(PrintedNameBuf);
+          NamedD->printName(PrintedNameOS);
+          auto Last = ResWithScope.rfind(PrintedNameOS.str());
+          if (Last != std::string::npos) {
+            std::string Res = ResWithScope.substr(0, Last);
+            if (Res.length() > 2 &&
+                Res.substr(Res.length() - 2, Res.length()) == "::")
+              Res = Res.substr(0, Res.length() - 2);
+
+            if (!Res.empty()) {
+              MarkedString Info = {"", "C++", "In " + Res};
+              H.contents.push_back(Info);
+            }
+          }
+        }
+      }
+    }
+    MarkedString MS = {"", "C++", Ref};
+    H.contents.push_back(MS);
+    return H;
+  }
+
+  if (!DeclLocationsFinder->getMacroInfos().empty()) {
+    const MacroInfo *MacroInf = DeclLocationsFinder->takeMacroInfos()[0];
+    if (MacroInf) {
+      SourceRange SR = SourceRange(MacroInf->getDefinitionLoc(),
+                                   MacroInf->getDefinitionEndLoc());
+      if (SR.isValid()) {
+        Location L = getDeclarationLocation(AST, SR);
+        H.range = L.range;
+        const FileEntry *FE = SourceMgr.getFileManager().getFile(L.uri.file);
+        FileID FID = SourceMgr.translateFile(FE);
+        Ref = SourceMgr.getBufferData(FID);
+        Start = SourceMgr.getFileOffset(SourceMgr.translateFileLineCol(
+            FE, L.range.start.line + 1, L.range.start.character + 1));
+        End = SourceMgr.getFileOffset(SourceMgr.translateFileLineCol(
+            FE, L.range.end.line + 1, L.range.end.character + 1));
+        Ref = Ref.slice(Start, End);
+      }
+    }
+    MarkedString MS = {"", "C++", "#define " + Ref.str()};
+    H.contents.push_back(MS);
+    return H;
+  }
+
+  return H;
 }
 
 void ParsedAST::ensurePreambleDeclsDeserialized() {
   if (PendingTopLevelDecls.empty())
     return;
 
   std::vector<const Decl *> Resolved;
   Resolved.reserve(PendingTopLevelDecls.size());
 
   ExternalASTSource &Source = *getASTContext().getExternalSource();
   for (serialization::DeclID TopLevelDecl : PendingTopLevelDecls) {
     // Resolve the declaration ID to an actual declaration, possibly
     // deserializing the declaration in the process.
     if (Decl *D = Source.GetExternalDecl(TopLevelDecl))
       Resolved.push_back(D);
   }
 
   TopLevelDecls.reserve(TopLevelDecls.size() + PendingTopLevelDecls.size());
   TopLevelDecls.insert(TopLevelDecls.begin(), Resolved.begin(), Resolved.end());
 
   PendingTopLevelDecls.clear();
 }
 
 ParsedAST::ParsedAST(ParsedAST &&Other) = default;
 
 ParsedAST &ParsedAST::operator=(ParsedAST &&Other) = default;
 
: Clang(std::move(Clang)), Action(std::move(Action)),
       Diags(std::move(Diags)), TopLevelDecls(std::move(TopLevelDecls)),
       PendingTopLevelDecls(std::move(PendingTopLevelDecls)) {
   assert(this->Clang);
   assert(this->Action);
 }
 
 ParsedASTWrapper::ParsedASTWrapper(ParsedASTWrapper &&Wrapper)
     : AST(std::move(Wrapper.AST)) {}
 
 ParsedASTWrapper::ParsedASTWrapper(llvm::Optional<ParsedAST> AST)
     : AST(std::move(AST)) {}
 
 PreambleData::PreambleData(PrecompiledPreamble Preamble,
                            std::vector<serialization::DeclID> TopLevelDeclIDs,
                            std::vector<DiagWithFixIts> Diags)
     : Preamble(std::move(Preamble)),
       TopLevelDeclIDs(std::move(TopLevelDeclIDs)), Diags(std::move(Diags)) {}
 
 std::shared_ptr<CppFile>
 CppFile::Create(PathRef FileName, tooling::CompileCommand Command,
                 bool StorePreamblesInMemory,
                 std::shared_ptr<PCHContainerOperations> PCHs,
                 clangd::Logger &Logger) {
   return std::shared_ptr<CppFile>(new CppFile(FileName, std::move(Command),
                                               StorePreamblesInMemory,
                                               std::move(PCHs), Logger));
 }
 
 CppFile::CppFile(PathRef FileName, tooling::CompileCommand Command,
                  bool StorePreamblesInMemory,
                  std::shared_ptr<PCHContainerOperations> PCHs,
                  clangd::Logger &Logger)
     : FileName(FileName), Command(std::move(Command)),
       StorePreamblesInMemory(StorePreamblesInMemory), RebuildCounter(0),
       RebuildInProgress(false), PCHs(std::move(PCHs)), Logger(Logger) {
 
   std::lock_guard<std::mutex> Lock(Mutex);
   LatestAvailablePreamble = nullptr;
   PreamblePromise.set_value(nullptr);
   PreambleFuture = PreamblePromise.get_future();
 
   ASTPromise.set_value(std::make_shared<ParsedASTWrapper>(llvm::None));
   ASTFuture = ASTPromise.get_future();
 }
 
 void CppFile::cancelRebuild() { deferCancelRebuild()(); }
 
 UniqueFunction<void()> CppFile::deferCancelRebuild() {
   std::unique_lock<std::mutex> Lock(Mutex);
   // Cancel an ongoing rebuild, if any, and wait for it to finish.
   unsigned RequestRebuildCounter = ++this->RebuildCounter;
   // Rebuild asserts that futures aren't ready if rebuild is cancelled.
   // We want to keep this invariant.
   if (futureIsReady(PreambleFuture)) {
     PreamblePromise = std::promise<std::shared_ptr<const PreambleData>>();
     PreambleFuture = PreamblePromise.get_future();
   }
auto FinishRebuild = [OldPreamble, VFS, RequestRebuildCounter, PCHs,
 
     std::unique_ptr<CompilerInvocation> CI;
     {
       // FIXME(ibiryukov): store diagnostics from CommandLine when we start
       // reporting them.
       EmptyDiagsConsumer CommandLineDiagsConsumer;
       IntrusiveRefCntPtr<DiagnosticsEngine> CommandLineDiagsEngine =
           CompilerInstance::createDiagnostics(new DiagnosticOptions,
                                               &CommandLineDiagsConsumer, false);
       CI = createCompilerInvocation(ArgStrs, CommandLineDiagsEngine, VFS);
     }
     assert(CI && "Couldn't create CompilerInvocation");
 
     std::unique_ptr<llvm::MemoryBuffer> ContentsBuffer =
         llvm::MemoryBuffer::getMemBufferCopy(NewContents, That->FileName);
 
     // A helper function to rebuild the preamble or reuse the existing one. Does
auto DoRebuildPreamble =
       StoreDiagsConsumer PreambleDiagnosticsConsumer(/*ref*/ PreambleDiags);
       IntrusiveRefCntPtr<DiagnosticsEngine> PreambleDiagsEngine =
           CompilerInstance::createDiagnostics(
               &CI->getDiagnosticOpts(), &PreambleDiagnosticsConsumer, false);
       CppFilePreambleCallbacks SerializedDeclsCollector;
       auto BuiltPreamble = PrecompiledPreamble::Build(
           *CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, VFS, PCHs,
           /*StoreInMemory=*/That->StorePreamblesInMemory,
           SerializedDeclsCollector);
 
       if (BuiltPreamble) {
         return std::make_shared<PreambleData>(
             std::move(*BuiltPreamble),
             SerializedDeclsCollector.takeTopLevelDeclIDs(),
             std::move(PreambleDiags));
       } else {
         return nullptr;
CppFile::RebuildGuard::~RebuildGuard() {
   Lock.unlock();
   File.RebuildCond.notify_all();
 }
 
 SourceLocation clangd::getBeginningOfIdentifier(ParsedAST &Unit,
                                                 const Position &Pos,
                                                 const FileEntry *FE) {
   // The language server protocol uses zero-based line and column numbers.
   // Clang uses one-based numbers.
 
   const ASTContext &AST = Unit.getASTContext();
   const SourceManager &SourceMgr = AST.getSourceManager();
 
   SourceLocation InputLocation =
       getMacroArgExpandedLocation(SourceMgr, FE, Pos);
   if (Pos.character == 0) {
     return InputLocation;
   }
 
   // This handle cases where the position is in the middle of a token or right
   // after the end of a token. In theory we could just use GetBeginningOfToken
   // to find the start of the token at the input position, but this doesn't
   // work when right after the end, i.e. foo|.
   // So try to go back by one and see if we're still inside the an identifier
   // token. If so, Take the beginning of this token.
   // (It should be the same identifier because you can't have two adjacent
   // identifiers without another token in between.)
   SourceLocation PeekBeforeLocation = getMacroArgExpandedLocation(
       SourceMgr, FE, Position{Pos.line, Pos.character - 1});
   Token Result;
   if (Lexer::getRawToken(PeekBeforeLocation, Result, SourceMgr,
                          AST.getLangOpts(), false)) {
     // getRawToken failed, just use InputLocation.
     return InputLocation;
   }
 
   if (Result.is(tok::raw_identifier)) {
     return Lexer::GetBeginningOfToken(PeekBeforeLocation, SourceMgr,
                                       AST.getLangOpts());
   }
 
   return InputLocation;
 }

clangd/Protocol.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_PROTOCOL_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PROTOCOL_H
 
 #include "JSONExpr.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/YAMLParser.h"
 #include <string>
 #include <vector>
 
 namespace clang {
 namespace clangd {
 
 class Logger;
 
 enum class ErrorCode {
static llvm::Optional<Range> parse(llvm::yaml::MappingNode *Params,
                                      clangd::Logger &Logger);
   static json::Expr unparse(const Range &P);
 };
 
 struct Location {
   /// The text document's URI.
   URI uri;
   Range range;
 
   friend bool operator==(const Location &LHS, const Location &RHS) {
     return LHS.uri == RHS.uri && LHS.range == RHS.range;
   }
 
   friend bool operator!=(const Location &LHS, const Location &RHS) {
     return !(LHS == RHS);
   }
 
struct TextDocumentPositionParams {
 
   /// The position inside the text document.
   Position position;
 
   static llvm::Optional<TextDocumentPositionParams>
   parse(llvm::yaml::MappingNode *Params, clangd::Logger &Logger);
 };
 
+struct MarkedString {
+  /**
+   * MarkedString can be used to render human readable text. It is either a
+   * markdown string
+   * or a code-block that provides a language and a code snippet. The language
+   * identifier
+   * is sematically equal to the optional language identifier in fenced code
+   * blocks in GitHub
+   * issues. See
+   * https://help.github.com/articles/creating-and-highlighting-code-blocks/#syntax-highlighting
+   *
+   * The pair of a language and a value is an equivalent to markdown:
+   * ```
+   * ${language}
+   * ${value}
+   * ```
+   *
+   * Note that markdown strings will be sanitized - that means html will be
+   * escaped.
+   */
+
+  std::string markdownString;
+  std::string language;
+  std::string codeBlockValue;
+
+  static json::Expr unparse(const MarkedString &MS);
+};
+
+struct Hover {
+
+  Hover(std::vector<MarkedString> contents, Range r)
+      : contents(contents), range(r) {}
+
+  /**
+   * The hover's content
+   */
+  std::vector<MarkedString> contents;
+
+  /**
+   * An optional range is a range inside a text document
+   * that is used to visualize a hover, e.g. by changing the background color.
+   */
+  llvm::Optional<Range> range;
+
+  static json::Expr unparse(const Hover &H);
+};
+
 /// The kind of a completion entry.
 enum class CompletionItemKind {
   Missing = 0,
   Text = 1,
   Method = 2,
   Function = 3,
   Constructor = 4,
   Field = 5,
   Variable = 6,
   Class = 7,
   Interface = 8,
   Module = 9,
   Property = 10,
   Unit = 11,
   Value = 12,
   Enum = 13,
   Keyword = 14,
   Snippet = 15,
   Color = 16,
   File = 17,
   Reference = 18,
 };
 
 /// Defines whether the insert text in a completion item should be interpreted
 /// as plain text or a snippet.
 enum class InsertTextFormat {
   Missing = 0,
   /// The primary text to be inserted is treated as a plain string.
   PlainText = 1,
   /// The primary text to be inserted is treated as a snippet.
   ///
   /// A snippet can define tab stops and placeholders with `$1`, `$2`
   /// and `${3:foo}`. `$0` defines the final tab stop, it defaults to the end
   /// of the snippet. Placeholders with equal identifiers are linked, that is
   /// typing in one will update others too.
   ///
   /// See also:
   /// https//github.com/Microsoft/vscode/blob/master/src/vs/editor/contrib/snippet/common/snippet.md
   Snippet = 2,
 };
 
 struct CompletionItem {
   /// The label of this completion item. By default also the text that is
   /// inserted when selecting this completion.
   std::string label;

clangd/Protocol.cpp

if (KeyValue == "textDocument") {
       Result.position = std::move(*Parsed);
     } else {
       logIgnoredField(KeyValue, Logger);
     }
   }
   return Result;
 }
 
+json::Expr Hover::unparse(const Hover &H) {
+  if (H.range.hasValue())
+  {
+    return json::obj{
+        {"contents", json::ary(H.contents)},
+        {"range", H.range.getValue()},
+    };
+  } else {
+    //Default Hover values
+    std::vector<MarkedString> EmptyVector;
+    Position BeginPosition = {0,0};
+    Position EndPosition = {0,0};
+    Range DefaultRange  = {BeginPosition, EndPosition};
+    Hover H = {EmptyVector, DefaultRange};
+    return json::obj{
+            {"contents", json::ary(H.contents)},
+            {"range", DefaultRange},
+        };
+  }
+}
+
+json::Expr MarkedString::unparse(const MarkedString &MS) {
+  return json::obj{
+        {"language", MS.language},
+        {"value", MS.codeBlockValue},
+    };
+}
+
 json::Expr CompletionItem::unparse(const CompletionItem &CI) {
   assert(!CI.label.empty() && "completion item label is required");
   json::obj Result{{"label", CI.label}};
   if (CI.kind != CompletionItemKind::Missing)
     Result["kind"] = static_cast<int>(CI.kind);
   if (!CI.detail.empty())
     Result["detail"] = CI.detail;
   if (!CI.documentation.empty())

clangd/ProtocolHandlers.h

public:
   virtual void onCodeAction(Ctx C, CodeActionParams &Params) = 0;
   virtual void onCompletion(Ctx C, TextDocumentPositionParams &Params) = 0;
   virtual void onSignatureHelp(Ctx C, TextDocumentPositionParams &Params) = 0;
   virtual void onGoToDefinition(Ctx C, TextDocumentPositionParams &Params) = 0;
   virtual void onSwitchSourceHeader(Ctx C, TextDocumentIdentifier &Params) = 0;
   virtual void onFileEvent(Ctx C, DidChangeWatchedFilesParams &Params) = 0;
   virtual void onCommand(Ctx C, ExecuteCommandParams &Params) = 0;
   virtual void onRename(Ctx C, RenameParams &Parames) = 0;
+  virtual void onCodeHover(Ctx C, TextDocumentPositionParams &Params) = 0;
 };
 
 void registerCallbackHandlers(JSONRPCDispatcher &Dispatcher, JSONOutput &Out,
                               ProtocolCallbacks &Callbacks);
 
 } // namespace clangd
 } // namespace clang
 
 #endif

clangd/ProtocolHandlers.cpp

struct HandlerRegisterer {
 
   JSONRPCDispatcher &Dispatcher;
   JSONOutput *Out;
   ProtocolCallbacks *Callbacks;
 };
 
 } // namespace
 
 void clangd::registerCallbackHandlers(JSONRPCDispatcher &Dispatcher,
                                       JSONOutput &Out,
                                       ProtocolCallbacks &Callbacks) {
   HandlerRegisterer Register{Dispatcher, &Out, &Callbacks};
 
   Register("initialize", &ProtocolCallbacks::onInitialize);
   Register("shutdown", &ProtocolCallbacks::onShutdown);
   Register("exit", &ProtocolCallbacks::onExit);
   Register("textDocument/didOpen", &ProtocolCallbacks::onDocumentDidOpen);
   Register("textDocument/didClose", &ProtocolCallbacks::onDocumentDidClose);
   Register("textDocument/didChange", &ProtocolCallbacks::onDocumentDidChange);
   Register("textDocument/rangeFormatting",
            &ProtocolCallbacks::onDocumentRangeFormatting);
   Register("textDocument/onTypeFormatting",
            &ProtocolCallbacks::onDocumentOnTypeFormatting);
   Register("textDocument/formatting", &ProtocolCallbacks::onDocumentFormatting);
   Register("textDocument/codeAction", &ProtocolCallbacks::onCodeAction);
   Register("textDocument/completion", &ProtocolCallbacks::onCompletion);
   Register("textDocument/signatureHelp", &ProtocolCallbacks::onSignatureHelp);
   Register("textDocument/definition", &ProtocolCallbacks::onGoToDefinition);
   Register("textDocument/switchSourceHeader",
            &ProtocolCallbacks::onSwitchSourceHeader);
   Register("textDocument/rename", &ProtocolCallbacks::onRename);
+  Register("textDocument/hover", &ProtocolCallbacks::onCodeHover);
   Register("workspace/didChangeWatchedFiles", &ProtocolCallbacks::onFileEvent);
   Register("workspace/executeCommand", &ProtocolCallbacks::onCommand);
 }

test/clangd/hover.test

+# RUN: clangd -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.
+#
+Content-Length: 125
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+
+Content-Length: 407
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"#define MACRO 1\nnamespace ns1 {\nint test = 5;\nstruct MyClass {\nint xasd;\nvoid anotherOperation() {\n}\nstatic int foo(MyClass*) {\nreturn 0;\n}\n\n};}\nint main() {\nint a;\na;\nint b = ns1::test;\nns1::MyClass* Params;\nParams->anotherOperation();\nMACRO;}\n"}}}
+
+Content-Length: 144
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":0,"character":12}}}
+# Go to local variable
+# CHECK: {"id":1,"jsonrpc":"2.0","result":{"contents":[{"language":"C++","value":"#define MACRO 1"}],"range":{"end":{"character":15,"line":0},"start":{"character":8,"line":0}}}}
+
+Content-Length: 144
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":14,"character":1}}}
+# Go to local variable
+# CHECK: {"id":1,"jsonrpc":"2.0","result":{"contents":[{"language":"C++","value":"int a"}],"range":{"end":{"character":5,"line":13},"start":{"character":0,"line":13}}}}
+
+Content-Length: 145
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":15,"character":15}}}
+# Go to local variable
+# CHECK: {"id":1,"jsonrpc":"2.0","result":{"contents":[{"language":"C++","value":"In ns1"},{"language":"C++","value":"int test = 5"}],"range":{"end":{"character":12,"line":2},"start":{"character":0,"line":2}}}}
+
+Content-Length: 145
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":16,"character":10}}}
+# Go to local variable
+# CHECK: {"id":1,"jsonrpc":"2.0","result":{"contents":[{"language":"C++","value":"In ns1"},{"language":"C++","value":"struct MyClass {"}],"range":{"end":{"character":16,"line":3},"start":{"character":0,"line":3}}}}
+
+Content-Length: 145
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":17,"character":13}}}
+# Go to local variable
+# CHECK: {"id":1,"jsonrpc":"2.0","result":{"contents":[{"language":"C++","value":"In ns1::MyClass"},{"language":"C++","value":"void anotherOperation() {"}],"range":{"end":{"character":25,"line":5},"start":{"character":0,"line":5}}}}
+
+Content-Length: 145
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":18,"character":1}}}
+# Go to local variable
+# CHECK: {"id":1,"jsonrpc":"2.0","result":{"contents":[{"language":"C++","value":"#define MACRO 1"}],"range":{"end":{"character":15,"line":0},"start":{"character":8,"line":0}}}}
+
+Content-Length: 44
+
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+
+

test/clangd/initialize-params-invalid.test

 # CHECK-NEXT:        "moreTriggerCharacter": []
 # CHECK-NEXT:      },
 # CHECK-NEXT:      "documentRangeFormattingProvider": true,
 # CHECK-NEXT:      "executeCommandProvider": {
 # CHECK-NEXT:        "commands": [
 # CHECK-NEXT:          "clangd.applyFix"
 # CHECK-NEXT:        ]
 # CHECK-NEXT:      },
+# CHECK-NEXT:    "hoverProvider": true,
 # CHECK-NEXT:      "renameProvider": true,
 # CHECK-NEXT:      "signatureHelpProvider": {
 # CHECK-NEXT:        "triggerCharacters": [
 # CHECK-NEXT:          "(",
 # CHECK-NEXT:          ","
 # CHECK-NEXT:        ]
 # CHECK-NEXT:      },
 # CHECK-NEXT:      "textDocumentSync": 1
 # CHECK-NEXT:    }
 # CHECK-NEXT:  }
+
 Content-Length: 44
 
 {"jsonrpc":"2.0","id":3,"method":"shutdown"}
 Content-Length: 33
 
 {"jsonrpc":"2.0":"method":"exit"}

test/clangd/initialize-params.test

 # RUN: clangd -pretty -run-synchronously < %s | FileCheck -strict-whitespace %s
 # It is absolutely vital that this file has CRLF line endings.
 #
 # Test initialize request parameters with rootUri
 Content-Length: 143
 
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootUri":"file:///path/to/workspace","capabilities":{},"trace":"off"}}
+
 #      CHECK:  "id": 0,
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": {
 # CHECK-NEXT:    "capabilities": {
 # CHECK-NEXT:      "codeActionProvider": true,
 # CHECK-NEXT:      "completionProvider": {
 # CHECK-NEXT:        "resolveProvider": false,
 # CHECK-NEXT:        "triggerCharacters": [

 # CHECK-NEXT:        "moreTriggerCharacter": []
 # CHECK-NEXT:      },
 # CHECK-NEXT:      "documentRangeFormattingProvider": true,
 # CHECK-NEXT:      "executeCommandProvider": {
 # CHECK-NEXT:        "commands": [
 # CHECK-NEXT:          "clangd.applyFix"
 # CHECK-NEXT:        ]
 # CHECK-NEXT:      },
+# CHECK-NEXT:    "hoverProvider": true,
 # CHECK-NEXT:      "renameProvider": true,
 # CHECK-NEXT:      "signatureHelpProvider": {
 # CHECK-NEXT:        "triggerCharacters": [
 # CHECK-NEXT:          "(",
 # CHECK-NEXT:          ","
 # CHECK-NEXT:        ]
 # CHECK-NEXT:      },
 # CHECK-NEXT:      "textDocumentSync": 1