[clangd] Add Macro Expansion to Hover

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.

Differential Revision: https://reviews.llvm.org/D127082

diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index 61c4d06..2ef6104 100644 (file)
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -666,7 +666,8 @@ getPredefinedExprHoverContents(const PredefinedExpr &PE, ASTContext &Ctx,
 }
 
 /// 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);
@@ -697,6 +698,29 @@ HoverInfo getHoverContents(const DefinedMacro &Macro, ParsedAST &AST) {
                 .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;
 }
 
@@ -1028,7 +1052,7 @@ llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
       // 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) {
@@ -1079,11 +1103,15 @@ llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
   if (!HI)
     return llvm::None;
 
-  auto Replacements = format::reformat(
-      Style, HI->Definition, tooling::Range(0, HI->Definition.size()));
-  if (auto Formatted =
-          tooling::applyAllReplacements(HI->Definition, Replacements))
-    HI->Definition = *Formatted;
+  // 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);
 
@@ -1178,25 +1206,31 @@ markup::Document HoverInfo::present() const {
 
   if (!Definition.empty()) {
     Output.addRuler();
-    std::string ScopeComment;
-    // Drop trailing "::".
-    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 =
-          "// In " + llvm::StringRef(LocalScope).rtrim(':').str() + '\n';
-    } else if (NamespaceScope && !NamespaceScope->empty()) {
-      ScopeComment = "// In namespace " +
-                     llvm::StringRef(*NamespaceScope).rtrim(':').str() + '\n';
+    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.
+        Buffer +=
+            "// In " + llvm::StringRef(LocalScope).rtrim(':').str() + '\n';
+      } else if (NamespaceScope && !NamespaceScope->empty()) {
+        Buffer += "// In namespace " +
+                  llvm::StringRef(*NamespaceScope).rtrim(':').str() + '\n';
+      }
+
+      if (!AccessSpecifier.empty()) {
+        Buffer += AccessSpecifier + ": ";
+      }
+
+      Buffer += Definition;
     }
-    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);
+
+    Output.addCodeBlock(Buffer, DefinitionLanguage);
   }
 
   return Output;

diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index d9596dc..663ebf2 100644 (file)
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -505,15 +505,60 @@ class Foo final {})cpp";
          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
@@ -1593,7 +1638,9 @@ TEST(Hover, All) {
           [](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
@@ -1604,6 +1651,8 @@ TEST(Hover, All) {
             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
@@ -1617,7 +1666,10 @@ TEST(Hover, All) {
             HI.Kind = index::SymbolKind::Macro;
             HI.Definition =
                 R"cpp(#define MACRO                                                                  \
-  { return 0; })cpp";
+  { return 0; }
+
+// Expands to
+{ return 0; })cpp";
           }},
       {
           R"cpp(// Forward class declaration
@@ -2991,6 +3043,21 @@ 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";