[clangd] Add Macro Expansion to Hover

[daiyousei-qz]

Summary

This patch adds macro expansion preview to hover info. Basically, the refactor infrastructure for expanding macro is used for this purpose. The following steps are added to getHoverContents for macros:

1. calling AST.getTokens().expansionStartingAt(...) to get expanded tokens
2. calling reformat(...) to format expanded tokens

Some opinions are wanted:

1. Should we present macro expansion before definition in the hover card?
2. Should we truncate/ignore macro expansion if it's too long? For performance and presentation reason, it might not be a good idea to expand pages worth of tokens in hover card. If so, what's the preferred threshold?

Also, some limitation applies:

1. Expansion isn't available in macro definition/arguments as the refactor code action isn't either.

[daiyousei-qz]

To elaborate on known situation where expansion isn't available:

1. expansion doesn't work on site of #define
2. expansion doesn't work for _Pragma(...) and alike
3. expansion doesn't work with builtin macro
4. expansion doesn't work for macros as arguments of function-like macro

Basically, anywhere the same code action isn't available. Need this be documented somewhere?

[nridge]

Thank you for working on this! I've been using clangd with this patch applied for a week or so, and it seems to be working well, I haven't run into any issues.

Could you please upload a patch with context, to make it easier to review? (Currently, above each hunk it says "Context not available", which makes it harder to find what code is being changed.)

Should we present macro expansion before definition in the hover card?

Yes, I think putting the expansion first would make sense, because it's more relevant (since it's specific to the invocation).

Should we truncate/ignore macro expansion if it's too long? For performance and presentation reason, it might not be a good idea to expand pages worth of tokens in hover card. If so, what's the preferred threshold?

I think a good client will manage longer content by e.g. making the hover scrollable, so we probably don't need a very restrictive limit, but I guess we should have some limit. Maybe something like 100 lines?

[nridge]

In D127082#3560642, @daiyousei-qz wrote:

Basically, anywhere the same code action isn't available. Need this be documented somewhere?

If you'd like, we could mention the ability to show macro expansions here, maybe with a screenshot, and then we could list the limitations in the same place.

The repository containing this website content is at https://github.com/llvm/clangd-www/blob/main/features.md

[daiyousei-qz]

• add context to patch
• use a fixed size buffer for expansion text

[daiyousei-qz]

Sorry I'm a little occupied lately.

Regarding to the order of Definition and MacroExpansion, I experimented putting expansion first and noticed the following:

1. Short definition and short expansion: order doesn't matter, it's always clear
2. Short definition and long expansion: putting definition first may have advantage in providing a general overview about the macro
3. Long definition and long expansion: hover card isn't a great place to read even if it's scrollable. I'd rather jump to definition or use refactor feature to expand the macro in place :(

Therefore, I still put definition in front of the expansion in this update.

Regarding to the size limit of expansion:
Here I'm using a 2048 bytes buffer. If exceeded, no macro expansion will be displayed. It might be hard to limit size in term of lines because the expanded segment of code will be reformated.

[daiyousei-qz]

use a rebased patch

[nridge]

Thanks for the update!

In D127082#3606746, @daiyousei-qz wrote:

Therefore, I still put definition in front of the expansion in this update.

Regarding to the size limit of expansion:
Here I'm using a 2048 bytes buffer.

I think these are reasonable choices to start with. We can always tweak them after the fact if we'd like.

clang-tools-extra/clangd/Hover.cpp

701

nit: these two lines can be combined as:

if (auto Expansion = AST.getTokens().expansionStartingAt(&Tok)) {
   ...
}

1115

It would be interesting to have a couple of test cases that exercise the reformatting in non-trivial ways, e.g. long expansions that need to be wrapped onto multiple lines

I would suggest two such test cases, one with the expansion being in a declaration context, and the second an expression context (for this one, to make it long enough, the expansion could contain e.g. an a ? b : c expression)

(I'm suggesting the expression-context testcase in part as a sanity check to make sure that format::reformat() handles such code reasonably in the first place)

[nridge]

clang-tools-extra/clangd/Hover.cpp

1115

Phabricator's inter-diff seems to be broken, but glancing through the new revision, it seems like this comment about adding a couple of more test cases hasn't been addressed yet, are you planning to do that in another update?

[daiyousei-qz]

It looks like my inline comment wasn't submitted (didn't click the submit button on the bottom). Here's my old comment.

Basically, I think if tests are going to be added for the text format, it should be done on format::reformat instead of hover.

clang-tools-extra/clangd/Hover.cpp

1115

Somehow, this comment goes out of the position.

In my opinion, such test should be written against format::reformat() directly instead of hover message in clangd. One problem is that we are using the current style in users' workspace to reformat the definition/expansion, which means the same tokens might present differently given different .clang-format or fallback style that the user has specified. I do agree that, if tokens don't conform a regular c++ expression, like ) . field, the presentation could be bad. But I suppose there's no obvious solution for that.

[nridge]

I would like to apply the updated patch to try it some more, but arc patch is failing for me.

Applying the raw diff manually with patch also fails with the following errors:

$ patch -p1 < ../patches/macro-hover.patch
patching file clang-tools-extra/clangd/Hover.h
Hunk #1 FAILED at 71 (different line endings).
1 out of 1 hunk FAILED -- saving rejects to file clang-tools-extra/clangd/Hover.h.rej
patching file clang-tools-extra/clangd/Hover.cpp
Hunk #1 FAILED at 641 (different line endings).
Hunk #2 FAILED at 670 (different line endings).
Hunk #3 FAILED at 1004 (different line endings).
Hunk #4 FAILED at 1055 (different line endings).
Hunk #5 FAILED at 1175 (different line endings).
5 out of 5 hunks FAILED -- saving rejects to file clang-tools-extra/clangd/Hover.cpp.rej
patching file clang-tools-extra/clangd/unittests/HoverTests.cpp
Hunk #1 FAILED at 478 (different line endings).
Hunk #2 FAILED at 1070 (different line endings).
Hunk #3 FAILED at 1567 (different line endings).
Hunk #4 FAILED at 1577 (different line endings).
Hunk #5 FAILED at 1591 (different line endings).
Hunk #6 FAILED at 2625 (different line endings).
6 out of 6 hunks FAILED -- saving rejects to file clang-tools-extra/clangd/unittests/HoverTests.cpp.rej

I'm guessing related to Windows line endings?

[nridge]

I fixed it locally by running dos2unix on the patch before applying.

However, if you're able to, please update the patch to remove the Windows line endings so it can be applied with arc patch, otherwise the patch metadata is lost during manual application.

[nridge]

clang-tools-extra/clangd/Hover.cpp

1115

In my opinion, such test should be written against format::reformat() directly instead of hover message in clangd. One problem is that we are using the current style in users' workspace to reformat the definition/expansion, which means the same tokens might present differently given different .clang-format or fallback style that the user has specified. I do agree that, if tokens don't conform a regular c++ expression, like ) . field, the presentation could be bad. But I suppose there's no obvious solution for that.

Ok, fair enough. I did some manual testing of the updated patch and reformat() seems to have sane behaviour for expressions.

From my point of view, this patch is good to go (with the line endings fixed). However, I will defer approval to @sammccall in case he has additional feedback.

[daiyousei-qz]

fix line end to LF

[daiyousei-qz]

fix patch context

[sammccall]

Sorry, didn't realize this was waiting on me! I like the feature and the threshold seems like a good idea. People might find it inconsistent, but we'll have to see.
I like placing it after the macro definition, I think the cases where expansion is/isn't shown feel more consistent that way.

My main hesitation is that it's nice if the members of HoverInfo are more generic: it ensures some coherence between different types of hovers and makes it easier to reason about changes to the Hoverinfo->markdown.

Concretely I wonder whether we can get away with appending this to the definition as a single block like:

#define DOUBLE(X) X##X

// Expands to:
mm

clang-tools-extra/clangd/Hover.cpp

704

nit: "Buffer" is redundant here

709

nit: the logic here seems a bit fiddly and micro-optimized - a few reallocations to produce a hover card don't matter.

consider just:

string ExpansionText;
for (tok : Expanded) {
  ExpansionText += ExpandedTok.text(SM);
  if (ExpansionText.size() > 2048) {
    ExpansionText.clear();
    break;
  }
}

(or even extract a function so you can early-return "", which is clearer)

1237

I think the ruler + paragraph header might appear too heavyweight, would need to take a look.

I think we're missing tests for present().
(If this can become part of the definition, then no need for new tests)

1238

I think "Expands to" or "Expanded text" is clearer than "macro expansion" - expansion names the process rather than the result.

clang-tools-extra/clangd/unittests/HoverTests.cpp

508–532

nit: these are known as "object-like" macros, I don't really know why

517

this is redundant with the definition, ideally I think we'd omit one (the output is pretty heavyweight).

It's *almost* correct to omit the expansion for object-like macros, the problem with this is:

#define INDIRECT DOUBLE(y)
#define DOUBLE(X) X##X
int INDIRECT; // we *should* show expansion here

Still, this is probably rare enough that it's better to not show expansions for object-like macros on balance?

(It would be possible to actually check for nested expansions but I doubt it's worth the complexity, certainly not in this patch)

[daiyousei-qz]

resolve review comment

produce expansion even if definition isn't available

[daiyousei-qz]

I have also moved the expansion out of the location checking branch. I think it's to find out if a macro actually has a definition which isn't always the case especially for special macros like __FILE__.

Here's the current presentation.

clang-tools-extra/clangd/unittests/HoverTests.cpp

517

I suppose if we don't have good way to detect nested object-like macro, just leave both definition and expansion is a better idea since people could simply ignore the redundant part. Though I don't have a strong opinion on this and could change that if you really want.

[sammccall]

clang-tools-extra/clangd/Hover.cpp

1207

Sorry, I think I wasn't clear about what I was asking for...
Rather than merging HoverInfo::MacroExpansion and HoverInfo::Definition during present(), I'd really like to eliminate HoverInfo::MacroExpansion entirely, and have HoverInfo::Definition be:

#define FOO BAR

// Expands to
BAR

This isn't completely consistent with the way scope/access are handled, but those are somewhat more generic features and I'd rather avoid adding new HoverInfo members specific to macros.

(The final presentation does look good, so this is just about the intermediate HoverInfo struct)

clang-tools-extra/clangd/unittests/HoverTests.cpp

517

OK, I also don't feel strongly, so let's go with what you have now.

[daiyousei-qz]

clang-tools-extra/clangd/Hover.cpp

1207

I see. Though I'm not sure if putting both of them in a single string will cause problems when doing the format. That is:

#define MACRO token token token token token

// Expands to
token token token token token token token token token

as a whole will be reformatted by clang-format and I'm not sure if the overall layout could always be preserved. Let me do some expriment tomorrow.

[daiyousei-qz]

remove dedicated MacroExpansion variable (reuse Definition)
added empty MACRO test

[daiyousei-qz]

@sammccall Hello, is there any update required by me?

[sammccall]

Sorry for the delay :-( This looks great!

It looks like this is your first patch, so you don't have commit access. I can land the patch for you.
Can you provide an email address for attribution?

[daiyousei-qz]

In D127082#3770763, @sammccall wrote:

Sorry for the delay :-( This looks great!

It looks like this is your first patch, so you don't have commit access. I can land the patch for you.
Can you provide an email address for attribution?

I had a patch for clang which Nathan had merged for me. Probably it's because of the lack of the metadata since I was using the web interface to submit differentials. Anyway, here's the info:
Qingyuan Zheng [e-mail]

[vitalybuka]

Looks like it breaks
https://lab.llvm.org/buildbot/#/builders/236/builds/206
https://lab.llvm.org/buildbot/#/builders/237/builds/85
https://lab.llvm.org/buildbot/#/builders/238/builds/103

[daiyousei-qz]

In D127082#3776173, @vitalybuka wrote:

Looks like it breaks
https://lab.llvm.org/buildbot/#/builders/236/builds/206
https://lab.llvm.org/buildbot/#/builders/237/builds/85
https://lab.llvm.org/buildbot/#/builders/238/builds/103

I take a quick look and it seems the failing tests were timed out. Considering the context of the change, I would guess it might be caused by some huge, nested macro being expanded. But it's weird since the expansion should only happen for Hover. Unfortunately, I don't have any aarch64 device to reproduce this locally and investigate further. Please feel free to reach out to me if there's anything I could help.

[vitalybuka]

897.65s: Clangd Unit Tests :: ./ClangdTests/25/72

In D127082#3776206, @daiyousei-qz wrote:

In D127082#3776173, @vitalybuka wrote:

Looks like it breaks
https://lab.llvm.org/buildbot/#/builders/236/builds/206
https://lab.llvm.org/buildbot/#/builders/237/builds/85
https://lab.llvm.org/buildbot/#/builders/238/builds/103

I take a quick look and it seems the failing tests were timed out. Considering the context of the change, I would guess it might be caused by some huge, nested macro being expanded. But it's weird since the expansion should only happen for Hover. Unfortunately, I don't have any aarch64 device to reproduce this locally and investigate further. Please feel free to reach out to me if there's anything I could help.

Actually it could be 72142fbac496a66769e16c80b076216d7c449ab2 as well
@sammccall we may need to disable or revert broken tests

After the patch:
Slowest Tests:
--------------------------------------------------------------------------
897.65s: Clangd Unit Tests :: ./ClangdTests/25/72

Before the patch 
Slowest Tests:
--------------------------------------------------------------------------
157.84s: Clangd Unit Tests :: ./ClangdTests/DefineOutlineTest/QualifyReturnValue

[nridge]

@daiyousei-qz what is the current status of this patch? Is it ready to be merged again? (If so, I can do that for you.)

[daiyousei-qz]

In D127082#4089377, @nridge wrote:

@daiyousei-qz what is the current status of this patch? Is it ready to be merged again? (If so, I can do that for you.)

Honestly, I don't know. I looked at TOT llvm repo in github and it looks like this patch is already there. I guess no further action is needed?

[nridge]

In D127082#4101322, @daiyousei-qz wrote:

In D127082#4089377, @nridge wrote:

@daiyousei-qz what is the current status of this patch? Is it ready to be merged again? (If so, I can do that for you.)

Honestly, I don't know. I looked at TOT llvm repo in github and it looks like this patch is already there. I guess no further action is needed?

Ah, my bad, for some reason I thought this had been backed out. Never mind then, and thanks for this improvement!

clang-tools-extra/clangd/Hover.cpp

QualType StringType = Ctx.getIncompleteArrayType(
         Ctx.CharTy.withConst(), ArrayType::ArraySizeModifier::Normal,
         /*IndexTypeQuals=*/0);
     HI.Type = printType(StringType, Ctx, PP);
   }
   return HI;
 }
 
 /// Generate a \p Hover object given the macro \p MacroDecl.
-HoverInfo getHoverContents(const DefinedMacro &Macro, ParsedAST &AST) {
+HoverInfo getHoverContents(const DefinedMacro &Macro, const syntax::Token &Tok,
+                           ParsedAST &AST) {
   HoverInfo HI;
   SourceManager &SM = AST.getSourceManager();
   HI.Name = std::string(Macro.Name);
   HI.Kind = index::SymbolKind::Macro;
   // FIXME: Populate documentation
   // FIXME: Populate parameters
 
   // Try to get the full definition, not just the name
if (!Invalid) {
       unsigned StartOffset = SM.getFileOffset(StartLoc);
       unsigned EndOffset = SM.getFileOffset(EndLoc);
       if (EndOffset <= Buffer.size() && StartOffset < EndOffset)
         HI.Definition =
             ("#define " + Buffer.substr(StartOffset, EndOffset - StartOffset))
                 .str();
     }
   }
+
+  if (auto Expansion = AST.getTokens().expansionStartingAt(&Tok)) {
+    // We drop expansion that's longer than the threshold.
+    // For extremely long expansion text, it's not readable from hover card
+    // anyway.
+    std::string ExpansionText;
+    for (const auto &ExpandedTok : Expansion->Expanded) {
+      ExpansionText += ExpandedTok.text(SM);
+      ExpansionText += " ";
+      if (ExpansionText.size() > 2048) {
+        ExpansionText.clear();
+        break;
+      }
+    }
+
+    if (!ExpansionText.empty()) {
+      if (!HI.Definition.empty()) {
+        HI.Definition += "\n\n";
+      }
+      HI.Definition += "// Expands to\n";
+      HI.Definition += ExpansionText;
+    }
+  }
   return HI;
 }
 
 std::string typeAsDefinition(const HoverInfo::PrintedType &PType) {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
   OS << PType.Type;
   if (PType.AKA)
llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
   // Macros and deducedtype only works on identifiers and auto/decltype keywords
   // respectively. Therefore they are only trggered on whichever works for them,
   // similar to SelectionTree::create().
   for (const auto &Tok : TokensTouchingCursor) {
     if (Tok.kind() == tok::identifier) {
       // Prefer the identifier token as a fallback highlighting range.
       HighlightRange = Tok.range(SM).toCharRange(SM);
       if (auto M = locateMacroAt(Tok, AST.getPreprocessor())) {
-        HI = getHoverContents(*M, AST);
+        HI = getHoverContents(*M, Tok, AST);
         break;
       }
     } else if (Tok.kind() == tok::kw_auto || Tok.kind() == tok::kw_decltype) {
       if (auto Deduced = getDeducedType(AST.getASTContext(), Tok.location())) {
         HI = getDeducedTypeHoverContents(*Deduced, Tok, AST.getASTContext(), PP,
                                          Index);
         HighlightRange = Tok.range(SM).toCharRange(SM);
         break;
if (const SelectionTree::Node *N = ST.commonAncestor()) {
       // FIXME: support hovers for other nodes?
       //  - built-in types
     }
   }
 
   if (!HI)
     return llvm::None;
 
+  // Reformat Definition
+  if (!HI->Definition.empty()) {
     auto Replacements = format::reformat(
         Style, HI->Definition, tooling::Range(0, HI->Definition.size()));
     if (auto Formatted =
             tooling::applyAllReplacements(HI->Definition, Replacements))
       HI->Definition = *Formatted;
+  }
+
   HI->DefinitionLanguage = getMarkdownLanguage(AST.getASTContext());
   HI->SymRange = halfOpenToRange(SM, HighlightRange);
 
   return HI;
 }
 
 markup::Document HoverInfo::present() const {
   markup::Document Output;
 
if (CalleeArgInfo) {
     if (CallPassType->Converted && CalleeArgInfo->Type)
       OS << " (converted to " << CalleeArgInfo->Type->Type << ")";
     Output.addParagraph().appendText(OS.str());
   }
 
   if (!Documentation.empty())
     parseDocumentation(Documentation, Output);
 
   if (!Definition.empty()) {
     Output.addRuler();
-    std::string ScopeComment;
-    // Drop trailing "::".
+    std::string Buffer;
+
+    if (!Definition.empty()) {
+      // Append scope comment, dropping trailing "::".
+      // Note that we don't print anything for global namespace, to not annoy
+      // non-c++ projects or projects that are not making use of namespaces.
       if (!LocalScope.empty()) {
         // Container name, e.g. class, method, function.
         // We might want to propagate some info about container type to print
         // function foo, class X, method X::bar, etc.
-      ScopeComment =
+        Buffer +=
             "// In " + llvm::StringRef(LocalScope).rtrim(':').str() + '\n';
       } else if (NamespaceScope && !NamespaceScope->empty()) {
-      ScopeComment = "// In namespace " +
-                     llvm::StringRef(*NamespaceScope).rtrim(':').str() + '\n';
+        Buffer += "// In namespace " +
       }
-    std::string DefinitionWithAccess = !AccessSpecifier.empty()
-                                           ? AccessSpecifier + ": " + Definition
-                                           : Definition;
-    // Note that we don't print anything for global namespace, to not annoy
-    // non-c++ projects or projects that are not making use of namespaces.
-    Output.addCodeBlock(ScopeComment + DefinitionWithAccess,
-                        DefinitionLanguage);
+
+      if (!AccessSpecifier.empty()) {
+        Buffer += AccessSpecifier + ": ";
+      }
+
+      Buffer += Definition;
+    }
+
+    Output.addCodeBlock(Buffer, DefinitionLanguage);
   }
 
   return Output;
 }
 
 // If the backtick at `Offset` starts a probable quoted range, return the range
 // (including the quotes).
 llvm::Optional<llvm::StringRef> getBacktickQuoteRange(llvm::StringRef Line,
                                                       unsigned Offset) {
   assert(Line[Offset] == '`');
 
   // The open-quote is usually preceded by whitespace.
   llvm::StringRef Prefix = Line.substr(0, Offset);

clang-tools-extra/clangd/unittests/HoverTests.cpp

class Foo final {})cpp";
         }
         )cpp",
        [](HoverInfo &HI) {
          HI.Name = "auto";
          HI.Kind = index::SymbolKind::TypeAlias;
          HI.Definition = "Foo<int>";
        }},
 
-      // macro
+      // empty macro
+      {R"cpp(
+        #define MACRO
+        [[MAC^RO]]
+        )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "MACRO";
+         HI.Kind = index::SymbolKind::Macro;
+         HI.Definition = "#define MACRO";
+       }},
+
+      // object-like macro
+      {R"cpp(
+        #define MACRO 41
+        int x = [[MAC^RO]];
+        )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "MACRO";
+         HI.Kind = index::SymbolKind::Macro;
+         HI.Definition = "#define MACRO 41\n\n"
+                         "// Expands to\n"
+                         "41";
+       }},
+
+      // function-like macro
       {R"cpp(
         // Best MACRO ever.
-        #define MACRO(x,y,z) void foo(x, y, z);
+        #define MACRO(x,y,z) void foo(x, y, z)
         [[MAC^RO]](int, double d, bool z = false);
         )cpp",
        [](HoverInfo &HI) {
-         HI.Name = "MACRO", HI.Kind = index::SymbolKind::Macro,
-         HI.Definition = "#define MACRO(x, y, z) void foo(x, y, z);";
+         HI.Name = "MACRO";
+         HI.Kind = index::SymbolKind::Macro;
+         HI.Definition = "#define MACRO(x, y, z) void foo(x, y, z)\n\n"
+                         "// Expands to\n"
+                         "void foo(int, double d, bool z = false)";
+       }},
+
+      // nested macro
+      {R"cpp(
+        #define STRINGIFY_AUX(s) #s
+        #define STRINGIFY(s) STRINGIFY_AUX(s)
+        #define DECL_STR(NAME, VALUE) const char *v_##NAME = STRINGIFY(VALUE)
+        #define FOO 41
+
+        [[DECL^_STR]](foo, FOO);
+        )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "DECL_STR";
+         HI.Kind = index::SymbolKind::Macro;
+         HI.Definition = "#define DECL_STR(NAME, VALUE) const char *v_##NAME = "
+                         "STRINGIFY(VALUE)\n\n"
+                         "// Expands to\n"
+                         "const char *v_foo = \"41\"";
        }},
 
       // constexprs
       {R"cpp(
         constexpr int add(int a, int b) { return a + b; }
         int [[b^ar]] = add(1, 2);
         )cpp",
        [](HoverInfo &HI) {
const std::function<void(HoverInfo &)> ExpectedBuilder;
       {
           R"cpp(// Macro
             #define MACRO 0
             int main() { return ^[[MACRO]]; }
           )cpp",
           [](HoverInfo &HI) {
             HI.Name = "MACRO";
             HI.Kind = index::SymbolKind::Macro;
-            HI.Definition = "#define MACRO 0";
+            HI.Definition = "#define MACRO 0\n\n"
+                            "// Expands to\n"
+                            "0";
           }},
       {
           R"cpp(// Macro
             #define MACRO 0
             #define MACRO2 ^[[MACRO]]
           )cpp",
           [](HoverInfo &HI) {
             HI.Name = "MACRO";
             HI.Kind = index::SymbolKind::Macro;
             HI.Definition = "#define MACRO 0";
+            // NOTE MACRO doesn't have expansion since it technically isn't
+            // expanded here
           }},
       {
           R"cpp(// Macro
             #define MACRO {\
               return 0;\
             }
             int main() ^[[MACRO]]
           )cpp",
           [](HoverInfo &HI) {
             HI.Name = "MACRO";
             HI.Kind = index::SymbolKind::Macro;
             HI.Definition =
                 R"cpp(#define MACRO                                                                  \
-  { return 0; })cpp";
+  { return 0; }
+
+// Expands to
           }},
       {
           R"cpp(// Forward class declaration
             class Foo;
             class Foo {};
             [[F^oo]]* foo();
           )cpp",
           [](HoverInfo &HI) {

 Value = 3
 Passed as arg_a (converted to alias_int)
 
 // In test::Bar
 int foo = 3)",
       },
       {
           [](HoverInfo &HI) {
+            HI.Kind = index::SymbolKind::Macro;
+            HI.Name = "PLUS_ONE";
+            HI.Definition = "#define PLUS_ONE(X) (X+1)\n\n"
+                            "// Expands to\n"
+                            "(1 + 1)";
+          },
+          R"(macro PLUS_ONE
+
+#define PLUS_ONE(X) (X+1)
+
+// Expands to
+(1 + 1))",
+      },
+      {
+          [](HoverInfo &HI) {
             HI.Kind = index::SymbolKind::Variable;
             HI.Name = "foo";
             HI.Definition = "int foo = 3";
             HI.LocalScope = "test::Bar::";
             HI.Value = "3";
             HI.Type = "int";
             HI.CalleeArgInfo.emplace();
             HI.CalleeArgInfo->Name = "arg_a";