[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)
HighCommander4 wrote: > I'm planning to revise the patch to make the following changes: > >1. Put the new behaviour behind a config option (I'm thinking `Hover` --> > `ShowFields`) >2. Add C language mode tests >3. Use `PrintingCallbacks` instead of a `PrintingPolicy` flag These changes have been made in the latest version of the patch. https://github.com/llvm/llvm-project/pull/89557 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)
https://github.com/HighCommander4 ready_for_review https://github.com/llvm/llvm-project/pull/89557 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)
https://github.com/HighCommander4 updated
https://github.com/llvm/llvm-project/pull/89557
>From fcb2ac4c68554d9c708b3db779b5570ff94725e8 Mon Sep 17 00:00:00 2001
From: Nathan Ridge
Date: Sun, 21 Apr 2024 20:30:16 -0400
Subject: [PATCH] [clangd] Show struct fields and enum members in hovers
Fixes https://github.com/clangd/clangd/issues/959
---
clang-tools-extra/clangd/Config.h | 3 +
clang-tools-extra/clangd/ConfigCompile.cpp| 6 +
clang-tools-extra/clangd/ConfigFragment.h | 5 +-
clang-tools-extra/clangd/ConfigYAML.cpp | 4 +
clang-tools-extra/clangd/Hover.cpp| 41 +
.../clangd/unittests/ConfigYAMLTests.cpp | 13 ++
.../clangd/unittests/HoverTests.cpp | 157 +-
clang/include/clang/AST/PrettyPrinter.h | 13 ++
clang/lib/AST/DeclPrinter.cpp | 32 +++-
clang/unittests/AST/DeclPrinterTest.cpp | 34 +++-
10 files changed, 295 insertions(+), 13 deletions(-)
diff --git a/clang-tools-extra/clangd/Config.h
b/clang-tools-extra/clangd/Config.h
index 4371c80a6c5877..3b6658e7ff034f 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -136,6 +136,9 @@ struct Config {
struct {
/// Whether hover show a.k.a type.
bool ShowAKA = true;
+
+/// Whether to show struct fields
+bool ShowFields = false;
} Hover;
struct {
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp
b/clang-tools-extra/clangd/ConfigCompile.cpp
index 5bb2eb4a9f803f..714a04fc77a5fc 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -616,6 +616,12 @@ struct FragmentCompiler {
C.Hover.ShowAKA = ShowAKA;
});
}
+if (F.ShowFields) {
+ Out.Apply.push_back(
+ [ShowFields(**F.ShowFields)](const Params &, Config ) {
+C.Hover.ShowFields = ShowFields;
+ });
+}
}
void compile(Fragment::InlayHintsBlock &) {
diff --git a/clang-tools-extra/clangd/ConfigFragment.h
b/clang-tools-extra/clangd/ConfigFragment.h
index 7fa61108c78a05..ccbc412f4fc533 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -309,8 +309,11 @@ struct Fragment {
/// Describes hover preferences.
struct HoverBlock {
-/// Whether hover show a.k.a type.
+/// Whether hovers show a.k.a type.
std::optional> ShowAKA;
+
+/// Whether hovers show struct fields
+std::optional> ShowFields;
};
HoverBlock Hover;
diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp
b/clang-tools-extra/clangd/ConfigYAML.cpp
index ce09af819247ae..68e4d66763d16c 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++ b/clang-tools-extra/clangd/ConfigYAML.cpp
@@ -235,6 +235,10 @@ class Parser {
if (auto ShowAKA = boolValue(N, "ShowAKA"))
F.ShowAKA = *ShowAKA;
});
+Dict.handle("ShowFields", [&](Node ) {
+ if (auto ShowFields = boolValue(N, "ShowFields"))
+F.ShowFields = *ShowFields;
+});
Dict.parse(N);
}
diff --git a/clang-tools-extra/clangd/Hover.cpp
b/clang-tools-extra/clangd/Hover.cpp
index 06b949bc4a2b55..db4049402f8fc0 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -66,12 +66,53 @@ namespace clang {
namespace clangd {
namespace {
+// A PrintingCallbacks implementation that prints public fields of
+// record types and enum members. Meant to be used with TerseOutput=true.
+class ClangdPrintingCallbacks : public PrintingCallbacks {
+public:
+ virtual ~ClangdPrintingCallbacks() = default;
+
+ void summarizeTagDecl(raw_ostream , const TagDecl *D,
+const PrintingPolicy ) const override {
+if (!Config::current().Hover.ShowFields) {
+ Out << " {}";
+ return;
+}
+
+// Don't worry about the details of the whitespace, e.g. space vs. newline,
+// as the printed definition will be passed through clang-format before
+// being sent to the client.
+Out << " { ";
+const bool IsRecord = isa(D);
+for (DeclContext::decl_iterator Member = D->decls_begin(),
+MemberEnd = D->decls_end();
+ Member != MemberEnd; ++Member) {
+ if (isa(*Member) ||
+ (isa(*Member) &&
+ dyn_cast(*Member)->getAccess() == AS_public)) {
+Member->print(Out, PP, 1);
+if (IsRecord) {
+ Out << "; ";
+} else {
+ DeclContext::decl_iterator Next = Member;
+ ++Next;
+ if (Next != MemberEnd)
+Out << ", ";
+}
+ }
+}
+Out << "}";
+ }
+};
+
PrintingPolicy getPrintingPolicy(PrintingPolicy Base) {
+ static ClangdPrintingCallbacks Callbacks;
Base.AnonymousTagLocations = false;
Base.TerseOutput = true;
Base.PolishForDeclaration = true;
Base.ConstantsAsWritten = true;
Base.SuppressTemplateArgsInCXXConstructors = true;
+ Base.Callbacks =
return Base;
[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)
https://github.com/HighCommander4 converted_to_draft https://github.com/llvm/llvm-project/pull/89557 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)
HighCommander4 wrote: I'm planning to revise the patch to make the following changes: 1. Put the new behaviour behind a config option (I'm thinking `Hover` --> `ShowFields`) 2. Add C language mode tests 3. Use `PrintingCallbacks` instead of a `PrintingPolicy` flag https://github.com/llvm/llvm-project/pull/89557 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)
HighCommander4 wrote: > I have a few questions: > >1. Do we want an RFC in discourse for the changes in `DeclPrinter`? An RFC might be overkill, but asking for review from a clang code owner for changes to an interface such as `PrintingPolicy` or `PrintingCallbacks` is probably a good idea. (If the clang reviewer feels an RFC is warranted, we can do it then.) >2. Do you think we should also add some test cases to > `DeclPrinterTest.cpp`? Yep, good call! https://github.com/llvm/llvm-project/pull/89557 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)
HighCommander4 wrote: > > 3. Regarding the implementation approach, is it fine to add a flag to > > `PrintingPolicy` (which is a clang utility class used in a variety of > > places) for a clangd-specific use case like this? I did it this way because > > the alternative seemed to involve duplicating a bunch of code related to > > decl-printing, but I'm happy to explore alternatives if this is an issue. > > I feel like "print only public fields" is too specific for clangd use case, > and probably won't generalize to other callers at all. But I definitely agree > with all the concerns around duplicating code. Looks like we have some > `PrintingCallbacks`, maybe we can have something like `SummarizeTagDecl`, > which enables customizing what to put into the body, when printing a TagDecl > in terse mode? Thanks for the suggestion! Using `PrintingCallbacks` here sounds like a promising idea. https://github.com/llvm/llvm-project/pull/89557 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)
@@ -474,6 +477,17 @@ void DeclPrinter::VisitDeclContext(DeclContext *DC, bool
Indent) {
for (DeclContext::decl_iterator D = DC->decls_begin(), DEnd =
DC->decls_end();
D != DEnd; ++D) {
+// Print enum members and public struct fields when
+// PrintTagTypeContents=true. Only applicable when TerseOutput=true since
+// otherwise all members are printed.
+if (Policy.TerseOutput) {
+ assert(Policy.PrintTagTypeContents);
+ if (!(isa(*D) ||
+(isa(*D) &&
+ dyn_cast(*D)->getAccess() == AS_public)))
HighCommander4 wrote:
The patch does seem to work for me on C files, but adding a test to exercise
the code in C mode is definitely a good idea. Thanks!
https://github.com/llvm/llvm-project/pull/89557
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)
colin-grant-work wrote: > As not all editors let you interact with hover cards, and just seeing a bunch > of field names, without any extra info, sounds suboptimal to me... I'd like > to have a way for such people to keep using hover cards without definition > eating up the whole screen estate. This basically raises the question of what the function of a hover is, and I don't think I'd like to limit its scope to just type annotation, with all other information available either only by triggering autocompletion (a bit of a workaround that may require extra typing, even if all you want to do is get an idea about how a struct is structured) or by jumping to the definition. ClangD's behavior is also a bit inconsistent. In a hover, it will show documentation comments around the declaration, but not any of the content of the declaration. That means that it may eat up a lot of screen real estate if the documentation is extensive, but in the absence of documentation, it won't show much at all. Given that most terminal editors have either a gesture to show a hover or a gesture to dismiss one, or both, I'm not sure that avoiding using space for their sake really helps anybody. Given those premises, I'd like to suggest aiming to figure out whether there is some useful data about structs / enums / (maybe even) classes / functions that ClangD can straightforwardly derive and (compactly) display, and if so, what that is. Allowing the behavior to be configured makes a lot of sense, perhaps both booleanly and scalarly, i.e. whether to display such data at all, and if so, the maximum amount that should be displayed, and in my view should address most concerns about its impact on particular editors. https://github.com/llvm/llvm-project/pull/89557 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)
HighCommander4 wrote: > what about showing this kind of "summary" only when hovering over the > declaration of the symbol, rather than references? Did you mean the other way around (references and not declaration, since when you're at the declaration you're likely already looking at the fields in the code)? Otherwise I'm missing the motivation for not showing it on references, which I think is the main part of the use case (being able to see this information at a glance when you're in a different file). https://github.com/llvm/llvm-project/pull/89557 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)
kadircet wrote:
So my 2cents;
> 1. I know a [previous comment on the
> bug](https://github.com/clangd/clangd/issues/959#issuecomment-998927030)
> stated "We don't show bodies of classes/enums/functions etc by policy", but
> can we consider changing this policy in the more limited case of public
> struct fields and enum members? Like I mentioned in [this
> comment](https://github.com/clangd/clangd/issues/959#issuecomment-2068307365),
> the usefulness/verbosity tradeoff might be different in this subset of
> cases. Also cc @colin-grant-work in case you'd like to weigh in on this
> further.
I feel like a more focused approach could still be preferred. E.g. what about
showing this kind of "summary" only when hovering over the declaration of the
symbol, rather than references? (similar to the way we show size/alignment
info). As I believe showing `struct Foo {}` in hover response, when the cursor
is already at a struct's definition, isn't really useful.
For enabling it more generally, I still have some hesitations, as I am not sure
how useful that interaction is in general. As not all editors let you interact
with hover cards, and just seeing a bunch of field names, without any extra
info, sounds suboptimal to me. I'd still prefer going over those fields via
`Foo{}.^` code completion, which shows all the "public" info with extra context
like "documentation".
> 2. If we're willing to make this change, do we want it unconditionally, or
> behind a new config option in the `Hover` section? My personal opinion is
> that even in the case of a struct/enum with many members this is not
> particularly bothersome and would lean towards making it unconditional, but I
> am of course happy to put it behind a config option if that helps build a
> consensus for this.
As you might've sensed from the previous response, i'd definitely prefer a
config option. As most terminal-based editors don't really have "rich"
hovercard support, and even if we set the default to "verbose", i'd like to
have a way for such people to keep using hover cards without definition eating
up the whole screen estate.
> 3. Regarding the implementation approach, is it fine to add a flag to
> `PrintingPolicy` (which is a clang utility class used in a variety of places)
> for a clangd-specific use case like this? I did it this way because the
> alternative seemed to involve duplicating a bunch of code related to
> decl-printing, but I'm happy to explore alternatives if this is an issue.
I feel like "print only public fields" is too specific for clangd use case, and
probably won't generalize to other callers at all. But I definitely agree with
all the concerns around duplicating code. Looks like we have some
`PrintingCallbacks`, maybe we can have something like `SummarizeTagDecl`, which
enables customizing what to put into the body, when printing a TagDecl in terse
mode?
https://github.com/llvm/llvm-project/pull/89557
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)
https://github.com/zyn0217 commented: Thank you! I have a few questions: 1) Do we want an RFC in discourse for the changes in `DeclPrinter`? 2) Do you think we should also add some test cases to `DeclPrinterTest.cpp`? https://github.com/llvm/llvm-project/pull/89557 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)
@@ -474,6 +477,17 @@ void DeclPrinter::VisitDeclContext(DeclContext *DC, bool
Indent) {
for (DeclContext::decl_iterator D = DC->decls_begin(), DEnd =
DC->decls_end();
D != DEnd; ++D) {
+// Print enum members and public struct fields when
+// PrintTagTypeContents=true. Only applicable when TerseOutput=true since
+// otherwise all members are printed.
+if (Policy.TerseOutput) {
+ assert(Policy.PrintTagTypeContents);
zyn0217 wrote:
This assertion is doing nothing because we have exited early at the function
beginning...?
https://github.com/llvm/llvm-project/pull/89557
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)
@@ -474,6 +477,17 @@ void DeclPrinter::VisitDeclContext(DeclContext *DC, bool
Indent) {
for (DeclContext::decl_iterator D = DC->decls_begin(), DEnd =
DC->decls_end();
D != DEnd; ++D) {
+// Print enum members and public struct fields when
+// PrintTagTypeContents=true. Only applicable when TerseOutput=true since
+// otherwise all members are printed.
+if (Policy.TerseOutput) {
+ assert(Policy.PrintTagTypeContents);
+ if (!(isa(*D) ||
+(isa(*D) &&
+ dyn_cast(*D)->getAccess() == AS_public)))
zyn0217 wrote:
Do you have any tests on C codes? Looking around the codebase, I think
`AS_none` *might* be used in some cases wrt non-C++ codes?
https://github.com/llvm/llvm-project/pull/89557
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)
https://github.com/zyn0217 edited https://github.com/llvm/llvm-project/pull/89557 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)
HighCommander4 wrote: A few things I would appreciate feedback on: 1. I know a [previous comment on the bug](https://github.com/clangd/clangd/issues/959#issuecomment-998927030) stated "We don't show bodies of classes/enums/functions etc by policy", but can we consider changing this policy in the more limited case of public struct fields and enum members? Like I mentioned in [this comment](https://github.com/clangd/clangd/issues/959#issuecomment-2068307365), the usefulness/verbosity tradeoff might be different in this subset of cases. Also cc @colin-grant-work in case you'd like to weigh in on this further. 2. If we're willing to make this change, do we want it unconditionally, or behind a new config option in the `Hover` section? My personal opinion is that even in the case of a struct/enum with many members this is not particularly bothersome and would lean towards making it unconditional, but I am of course happy to put it behind a config option if that helps build a consensus for this. 3. Regarding the implementation approach, is it fine to add a flag to `PrintingPolicy` (which is a clang utility class used in a variety of places) for a clangd-specific use case like this? I did it this way because the alternative seemed to involve duplicating a bunch of code related to decl-printing, but I'm happy to explore alternatives if this is an issue. https://github.com/llvm/llvm-project/pull/89557 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)
llvmbot wrote:
@llvm/pr-subscribers-clangd
Author: Nathan Ridge (HighCommander4)
Changes
Fixes https://github.com/clangd/clangd/issues/959
---
Full diff: https://github.com/llvm/llvm-project/pull/89557.diff
4 Files Affected:
- (modified) clang-tools-extra/clangd/Hover.cpp (+2)
- (modified) clang-tools-extra/clangd/unittests/HoverTests.cpp (+13-4)
- (modified) clang/include/clang/AST/PrettyPrinter.h (+9-1)
- (modified) clang/lib/AST/DeclPrinter.cpp (+17-3)
``diff
diff --git a/clang-tools-extra/clangd/Hover.cpp
b/clang-tools-extra/clangd/Hover.cpp
index 06b949bc4a2b55..d7c433876b08bc 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -69,6 +69,8 @@ namespace {
PrintingPolicy getPrintingPolicy(PrintingPolicy Base) {
Base.AnonymousTagLocations = false;
Base.TerseOutput = true;
+ // Show struct fields and enum members.
+ Base.PrintTagTypeContents = true;
Base.PolishForDeclaration = true;
Base.ConstantsAsWritten = true;
Base.SuppressTemplateArgsInCXXConstructors = true;
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp
b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 35db757b9c15b5..8c19f5fb3b0ad0 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1645,7 +1645,16 @@ TEST(Hover, All) {
{
R"cpp(// Struct
namespace ns1 {
- struct MyClass {};
+ struct MyClass {
+// Public fields shown in hover
+int field1;
+int field2;
+
+// Methods and private fields not shown
+void method();
+ private:
+bool private_field;
+ };
} // namespace ns1
int main() {
ns1::[[My^Class]]* Params;
@@ -1655,7 +1664,7 @@ TEST(Hover, All) {
HI.Name = "MyClass";
HI.Kind = index::SymbolKind::Struct;
HI.NamespaceScope = "ns1::";
-HI.Definition = "struct MyClass {}";
+HI.Definition = "struct MyClass {\n int field1;\n int
field2;\n}";
}},
{
R"cpp(// Class
@@ -1685,7 +1694,7 @@ TEST(Hover, All) {
HI.Name = "MyUnion";
HI.Kind = index::SymbolKind::Union;
HI.NamespaceScope = "ns1::";
-HI.Definition = "union MyUnion {}";
+HI.Definition = "union MyUnion {\n int x;\n int y;\n}";
}},
{
R"cpp(// Function definition via pointer
@@ -2030,7 +2039,7 @@ TEST(Hover, All) {
HI.Name = "Hello";
HI.Kind = index::SymbolKind::Enum;
HI.NamespaceScope = "";
-HI.Definition = "enum Hello {}";
+HI.Definition = "enum Hello { ONE, TWO, THREE }";
HI.Documentation = "Enum declaration";
}},
{
diff --git a/clang/include/clang/AST/PrettyPrinter.h
b/clang/include/clang/AST/PrettyPrinter.h
index da276e26049b00..6b8f1ae9143df3 100644
--- a/clang/include/clang/AST/PrettyPrinter.h
+++ b/clang/include/clang/AST/PrettyPrinter.h
@@ -70,7 +70,7 @@ struct PrintingPolicy {
Restrict(LO.C99), Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
UseVoidForZeroParams(!LO.CPlusPlus),
SplitTemplateClosers(!LO.CPlusPlus11), TerseOutput(false),
-PolishForDeclaration(false), Half(LO.Half),
+PrintTagTypeContents(false), PolishForDeclaration(false),
Half(LO.Half),
MSWChar(LO.MicrosoftExt && !LO.WChar), IncludeNewlines(true),
MSVCFormatting(false), ConstantsAsWritten(false),
SuppressImplicitBase(false), FullyQualifiedName(false),
@@ -252,6 +252,14 @@ struct PrintingPolicy {
LLVM_PREFERRED_TYPE(bool)
unsigned TerseOutput : 1;
+ /// Print the contents of tag (i.e. record and enum) types, even with
+ /// TerseOutput=true.
+ ///
+ /// For record types (structs/classes/unions), this only prints public
+ /// data members. For enum types, this prints enumerators.
+ /// Has no effect if TerseOutput=false (which prints all members).
+ unsigned PrintTagTypeContents : 1;
+
/// When true, do certain refinement needed for producing proper declaration
/// tag; such as, do not print attributes attached to the declaration.
///
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index 43d221968ea3fb..3fe02fff687fe1 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -21,6 +21,7 @@
#include "clang/AST/ExprCXX.h"
#include "clang/AST/PrettyPrinter.h"
#include "clang/Basic/Module.h"
+#include "clang/Basic/Specifiers.h"
#include "llvm/Support/raw_ostream.h"
using namespace clang;
@@ -464,8 +465,10 @@ void
DeclPrinter::PrintConstructorInitializers(CXXConstructorDecl *CDecl,
//
void DeclPrinter::VisitDeclContext(DeclContext
[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)
https://github.com/HighCommander4 ready_for_review https://github.com/llvm/llvm-project/pull/89557 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Show struct fields and enum members in hovers (PR #89557)
https://github.com/HighCommander4 created
https://github.com/llvm/llvm-project/pull/89557
Fixes https://github.com/clangd/clangd/issues/959
>From d98c95bf213f0c6e81a46a9e37d376b855bb4867 Mon Sep 17 00:00:00 2001
From: Nathan Ridge
Date: Sun, 21 Apr 2024 20:30:16 -0400
Subject: [PATCH] [clangd] Show struct fields and enum members in hovers
Fixes https://github.com/clangd/clangd/issues/959
---
clang-tools-extra/clangd/Hover.cpp| 2 ++
.../clangd/unittests/HoverTests.cpp | 17
clang/include/clang/AST/PrettyPrinter.h | 10 +-
clang/lib/AST/DeclPrinter.cpp | 20 ---
4 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/clang-tools-extra/clangd/Hover.cpp
b/clang-tools-extra/clangd/Hover.cpp
index 06b949bc4a2b55..d7c433876b08bc 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -69,6 +69,8 @@ namespace {
PrintingPolicy getPrintingPolicy(PrintingPolicy Base) {
Base.AnonymousTagLocations = false;
Base.TerseOutput = true;
+ // Show struct fields and enum members.
+ Base.PrintTagTypeContents = true;
Base.PolishForDeclaration = true;
Base.ConstantsAsWritten = true;
Base.SuppressTemplateArgsInCXXConstructors = true;
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp
b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 35db757b9c15b5..8c19f5fb3b0ad0 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1645,7 +1645,16 @@ TEST(Hover, All) {
{
R"cpp(// Struct
namespace ns1 {
- struct MyClass {};
+ struct MyClass {
+// Public fields shown in hover
+int field1;
+int field2;
+
+// Methods and private fields not shown
+void method();
+ private:
+bool private_field;
+ };
} // namespace ns1
int main() {
ns1::[[My^Class]]* Params;
@@ -1655,7 +1664,7 @@ TEST(Hover, All) {
HI.Name = "MyClass";
HI.Kind = index::SymbolKind::Struct;
HI.NamespaceScope = "ns1::";
-HI.Definition = "struct MyClass {}";
+HI.Definition = "struct MyClass {\n int field1;\n int
field2;\n}";
}},
{
R"cpp(// Class
@@ -1685,7 +1694,7 @@ TEST(Hover, All) {
HI.Name = "MyUnion";
HI.Kind = index::SymbolKind::Union;
HI.NamespaceScope = "ns1::";
-HI.Definition = "union MyUnion {}";
+HI.Definition = "union MyUnion {\n int x;\n int y;\n}";
}},
{
R"cpp(// Function definition via pointer
@@ -2030,7 +2039,7 @@ TEST(Hover, All) {
HI.Name = "Hello";
HI.Kind = index::SymbolKind::Enum;
HI.NamespaceScope = "";
-HI.Definition = "enum Hello {}";
+HI.Definition = "enum Hello { ONE, TWO, THREE }";
HI.Documentation = "Enum declaration";
}},
{
diff --git a/clang/include/clang/AST/PrettyPrinter.h
b/clang/include/clang/AST/PrettyPrinter.h
index da276e26049b00..6b8f1ae9143df3 100644
--- a/clang/include/clang/AST/PrettyPrinter.h
+++ b/clang/include/clang/AST/PrettyPrinter.h
@@ -70,7 +70,7 @@ struct PrintingPolicy {
Restrict(LO.C99), Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
UseVoidForZeroParams(!LO.CPlusPlus),
SplitTemplateClosers(!LO.CPlusPlus11), TerseOutput(false),
-PolishForDeclaration(false), Half(LO.Half),
+PrintTagTypeContents(false), PolishForDeclaration(false),
Half(LO.Half),
MSWChar(LO.MicrosoftExt && !LO.WChar), IncludeNewlines(true),
MSVCFormatting(false), ConstantsAsWritten(false),
SuppressImplicitBase(false), FullyQualifiedName(false),
@@ -252,6 +252,14 @@ struct PrintingPolicy {
LLVM_PREFERRED_TYPE(bool)
unsigned TerseOutput : 1;
+ /// Print the contents of tag (i.e. record and enum) types, even with
+ /// TerseOutput=true.
+ ///
+ /// For record types (structs/classes/unions), this only prints public
+ /// data members. For enum types, this prints enumerators.
+ /// Has no effect if TerseOutput=false (which prints all members).
+ unsigned PrintTagTypeContents : 1;
+
/// When true, do certain refinement needed for producing proper declaration
/// tag; such as, do not print attributes attached to the declaration.
///
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index 43d221968ea3fb..3fe02fff687fe1 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -21,6 +21,7 @@
#include "clang/AST/ExprCXX.h"
#include "clang/AST/PrettyPrinter.h"
#include "clang/Basic/Module.h"
+#include "clang/Basic/Specifiers.h"
#include "llvm/Support/raw_ostream.h"
using namespace clang;
@@
