[clang-tools-extra] 44bbf20 - [clangd] Add Macro Expansion to Hover
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 7 08:50:18 PDT 2022
Author: Qingyuan Zheng
Date: 2022-09-07T17:49:45+02:00
New Revision: 44bbf20965d2c1e00bb805343ad80dbb7758bf3d
URL: https://github.com/llvm/llvm-project/commit/44bbf20965d2c1e00bb805343ad80dbb7758bf3d
DIFF: https://github.com/llvm/llvm-project/commit/44bbf20965d2c1e00bb805343ad80dbb7758bf3d.diff
LOG: [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
Added:
Modified:
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index 61c4d0658c91..2ef6104e3a43 100644
--- 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 d9596dc7e693..663ebf240373 100644
--- 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
@@ -2988,6 +3040,21 @@ 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) {
More information about the cfe-commits
mailing list