daiyousei-qz updated this revision to Diff 446041.
daiyousei-qz added a comment.
resolve review comment
produce expansion even if definition isn't available
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127082/new/
https://reviews.llvm.org/D127082
Files:
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/Hover.h
clang-tools-extra/clangd/unittests/HoverTests.cpp
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -478,15 +478,46 @@
HI.Definition = "Foo<int>";
}},
- // 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";
+ HI.MacroExpansion = "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)";
+ HI.MacroExpansion = "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)";
+ HI.MacroExpansion = "const char *v_foo = \"41\"";
}},
// constexprs
@@ -1070,6 +1101,7 @@
EXPECT_EQ(H->Kind, Expected.Kind);
EXPECT_EQ(H->Documentation, Expected.Documentation);
EXPECT_EQ(H->Definition, Expected.Definition);
+ EXPECT_EQ(H->MacroExpansion, Expected.MacroExpansion);
EXPECT_EQ(H->Type, Expected.Type);
EXPECT_EQ(H->ReturnType, Expected.ReturnType);
EXPECT_EQ(H->Parameters, Expected.Parameters);
@@ -1567,6 +1599,7 @@
HI.Name = "MACRO";
HI.Kind = index::SymbolKind::Macro;
HI.Definition = "#define MACRO 0";
+ HI.MacroExpansion = "0";
}},
{
R"cpp(// Macro
@@ -1577,6 +1610,8 @@
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
@@ -1591,6 +1626,7 @@
HI.Definition =
R"cpp(#define MACRO \
{ return 0; })cpp";
+ HI.MacroExpansion = "{ return 0; }";
}},
{
R"cpp(// Forward class declaration
@@ -2625,6 +2661,7 @@
EXPECT_EQ(H->Kind, Expected.Kind);
EXPECT_EQ(H->Documentation, Expected.Documentation);
EXPECT_EQ(H->Definition, Expected.Definition);
+ EXPECT_EQ(H->MacroExpansion, Expected.MacroExpansion);
EXPECT_EQ(H->Type, Expected.Type);
EXPECT_EQ(H->ReturnType, Expected.ReturnType);
EXPECT_EQ(H->Parameters, Expected.Parameters);
@@ -2986,6 +3023,20 @@
// 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)";
+ HI.MacroExpansion = "(1 + 1)";
+ },
+ R"(macro PLUS_ONE
+
+#define PLUS_ONE(X) (X+1)
+
+// Expands to
+(1 + 1))",
},
{
[](HoverInfo &HI) {
Index: clang-tools-extra/clangd/Hover.h
===================================================================
--- clang-tools-extra/clangd/Hover.h
+++ clang-tools-extra/clangd/Hover.h
@@ -71,6 +71,7 @@
std::string Documentation;
/// Source code containing the definition of the symbol.
std::string Definition;
+ std::string MacroExpansion;
const char *DefinitionLanguage = "cpp";
/// Access specifier for declarations inside class/struct/unions, empty for
/// others.
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -641,7 +641,8 @@
}
/// Generate a \p Hover object given the macro \p MacroDecl.
-HoverInfo getHoverContents(const DefinedMacro &Macro, ParsedAST &AST) {
+HoverInfo getHoverContents(const syntax::Token &Tok, const DefinedMacro &Macro,
+ ParsedAST &AST) {
HoverInfo HI;
SourceManager &SM = AST.getSourceManager();
HI.Name = std::string(Macro.Name);
@@ -672,6 +673,23 @@
.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;
+ }
+ }
+
+ HI.MacroExpansion = std::move(ExpansionText);
+ }
return HI;
}
@@ -1004,7 +1022,7 @@
// 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(Tok, *M, AST);
break;
}
} else if (Tok.kind() == tok::kw_auto || Tok.kind() == tok::kw_decltype) {
@@ -1055,11 +1073,25 @@
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;
+ }
+
+ // Reformat Macro Expansion
+ if (!HI->MacroExpansion.empty()) {
+ auto Replacements =
+ format::reformat(Style, HI->MacroExpansion,
+ tooling::Range(0, HI->MacroExpansion.size()));
+ if (auto Formatted =
+ tooling::applyAllReplacements(HI->MacroExpansion, Replacements))
+ HI->MacroExpansion = *Formatted;
+ }
+
HI->DefinitionLanguage = getMarkdownLanguage(AST.getASTContext());
HI->SymRange = halfOpenToRange(SM, HighlightRange);
@@ -1152,27 +1184,39 @@
if (!Documentation.empty())
parseDocumentation(Documentation, Output);
- if (!Definition.empty()) {
+ if (!Definition.empty() || !MacroExpansion.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';
+ }
+
+ // Append definition with access
+ Buffer += !AccessSpecifier.empty() ? AccessSpecifier + ": " + Definition
+ : 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);
+
+ if (!MacroExpansion.empty()) {
+ if (!Definition.empty()) {
+ // One to end Definition line, one as separator line
+ Buffer += "\n\n";
+ }
+ Buffer += "// Expands to\n" + MacroExpansion;
+ }
+
+ Output.addCodeBlock(Buffer, DefinitionLanguage);
}
return Output;
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
