[clangd] Show definition of underlying struct when hovering over a typedef #89570

HighCommander4 · 2024-04-22T06:25:12Z

Fixes clangd/clangd#2020

llvmbot · 2024-04-22T06:25:45Z

@llvm/pr-subscribers-clangd
@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang

Author: Nathan Ridge (HighCommander4)

Changes

Fixes clangd/clangd#2020

Full diff: https://github.com/llvm/llvm-project/pull/89570.diff

4 Files Affected:

(modified) clang-tools-extra/clangd/Hover.cpp (+12)
(modified) clang-tools-extra/clangd/unittests/HoverTests.cpp (+28-5)
(modified) clang/include/clang/AST/PrettyPrinter.h (+9-1)
(modified) clang/lib/AST/DeclPrinter.cpp (+17-3)

diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index 06b949bc4a2b55..598c2a68b7bf16 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;
@@ -150,6 +152,16 @@ std::string printDefinition(const Decl *D, PrintingPolicy PP,
   std::string Definition;
   llvm::raw_string_ostream OS(Definition);
   D->print(OS, PP);
+  
+  // For a typedef to a TagDecl, also print the underlying TagDecl
+  if (const auto *TND = dyn_cast<TypedefNameDecl>(D)) {
+    if (const auto *TT = dyn_cast<TagType>(
+            TND->getUnderlyingType().getCanonicalType().getTypePtr())) {
+      OS << ";\n";
+      TT->getDecl()->print(OS, PP);
+    }
+  }
+
   OS.flush();
   return Definition;
 }
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 35db757b9c15b5..9657869b1b7641 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,8 +1664,22 @@ 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(// Typedef to struct
+              struct Point { int x; int y; };
+              typedef Point TPoint;
+              [[TP^oint]] tp;
+            )cpp",
+            [](HoverInfo &HI) {
+              HI.Name = "TPoint";
+              HI.Kind = index::SymbolKind::TypeAlias;
+              HI.NamespaceScope = "";
+              HI.Type = "struct Point";
+              HI.Definition = "typedef Point TPoint;\nstruct Point {\n  int x;\n  int y;\n}";
+            }
+          },
       {
           R"cpp(// Class
             namespace ns1 {
@@ -1685,7 +1708,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
@@ -1880,7 +1903,7 @@ TEST(Hover, All) {
             HI.Name = "Foo";
             HI.Kind = index::SymbolKind::TypeAlias;
             HI.NamespaceScope = "";
-            HI.Definition = "typedef struct Bar Foo";
+            HI.Definition = "typedef struct Bar Foo;\nstruct Bar {}";
             HI.Type = "struct Bar";
             HI.Documentation = "Typedef with embedded definition";
           }},
@@ -2030,7 +2053,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 *DC, bool Indent) {
-  if (Policy.TerseOutput)
-    return;
+  if (Policy.TerseOutput) {
+    if (!Policy.PrintTagTypeContents || !isa<TagDecl>(DC))
+      return;
+  }
 
   if (Indent)
     Indentation += Policy.Indentation;
@@ -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<EnumConstantDecl>(*D) ||
+            (isa<FieldDecl>(*D) &&
+             dyn_cast<FieldDecl>(*D)->getAccess() == AS_public)))
+        continue;
+    }
+
     // Don't print ObjCIvarDecls, as they are printed when visiting the
     // containing ObjCInterfaceDecl.
     if (isa<ObjCIvarDecl>(*D))
@@ -1182,7 +1196,7 @@ void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) {
 
     // Print the class definition
     // FIXME: Doesn't print access specifiers, e.g., "public:"
-    if (Policy.TerseOutput) {
+    if (Policy.TerseOutput && !Policy.PrintTagTypeContents) {
       Out << " {}";
     } else {
       Out << " {\n";

HighCommander4 · 2024-04-22T06:26:23Z

(This builds on #89557 and the commit view shows both commits. I still need to figure out how to do proper stacked PRs...)

…pedef Fixes clangd/clangd#2020

HighCommander4 · 2024-04-25T22:58:34Z

Updated patch with the following changes:

Decoupled from C struct members and enum values are not shown on type hover clangd/clangd#959
Handle the case of multiple layers of typedefs, and add tests for this case
Add C language specific tests

github-actions · 2024-04-25T23:02:53Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 9617da88ab961145047076c45bb2bb1ac4513634 44aba390954c7b551ed7102e8e7b4209207c0d87 -- clang-tools-extra/clangd/Hover.cpp clang-tools-extra/clangd/unittests/HoverTests.cpp

View the diff from clang-format here.

diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 582a311f35..fbef81f781 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1657,35 +1657,34 @@ TEST(Hover, All) {
             HI.NamespaceScope = "ns1::";
             HI.Definition = "struct MyClass {}";
           }},
-          {
-            R"cpp(// Typedef to struct
+      {
+          R"cpp(// Typedef to struct
               struct Point { int x; int y; };
               typedef Point TPoint;
               [[TP^oint]] tp;
             )cpp",
-            [](HoverInfo &HI) {
-              HI.Name = "TPoint";
-              HI.Kind = index::SymbolKind::TypeAlias;
-              HI.NamespaceScope = "";
-              HI.Type = "struct Point";
-              HI.Definition = "typedef Point TPoint;\nstruct Point {}";
-            }
-          },
-          {
-            R"cpp(// Two layers of typedef
+          [](HoverInfo &HI) {
+            HI.Name = "TPoint";
+            HI.Kind = index::SymbolKind::TypeAlias;
+            HI.NamespaceScope = "";
+            HI.Type = "struct Point";
+            HI.Definition = "typedef Point TPoint;\nstruct Point {}";
+          }},
+      {
+          R"cpp(// Two layers of typedef
               struct Point { int x; int y; };
               typedef Point TPoint;
               typedef TPoint TTPoint;
               [[TTP^oint]] tp;
             )cpp",
-            [](HoverInfo &HI) {
-              HI.Name = "TTPoint";
-              HI.Kind = index::SymbolKind::TypeAlias;
-              HI.NamespaceScope = "";
-              HI.Type = "struct Point";
-              HI.Definition = "typedef TPoint TTPoint;\ntypedef Point TPoint;\nstruct Point {}";
-            }
-          },
+          [](HoverInfo &HI) {
+            HI.Name = "TTPoint";
+            HI.Kind = index::SymbolKind::TypeAlias;
+            HI.NamespaceScope = "";
+            HI.Type = "struct Point";
+            HI.Definition = "typedef TPoint TTPoint;\ntypedef Point "
+                            "TPoint;\nstruct Point {}";
+          }},
       {
           R"cpp(// Class
             namespace ns1 {
@@ -3158,35 +3157,34 @@ TEST(Hover, CLanguage) {
     const char *const Code;
     const std::function<void(HoverInfo &)> ExpectedBuilder;
   } Cases[] = {
-    {
-      R"cpp(// Typedef to struct
+      {
+          R"cpp(// Typedef to struct
         struct Point { int x; int y; };
         typedef struct Point TPoint;
         [[TP^oint]] tp;
       )cpp",
-      [](HoverInfo &HI) {
-        HI.Name = "TPoint";
-        HI.Kind = index::SymbolKind::TypeAlias;
-        HI.NamespaceScope = "";
-        HI.Type = "struct Point";
-        HI.Definition = "typedef struct Point TPoint;\nstruct Point {}";
-      }
-    },
-    {
-      R"cpp(// Two layers of typedef
+          [](HoverInfo &HI) {
+            HI.Name = "TPoint";
+            HI.Kind = index::SymbolKind::TypeAlias;
+            HI.NamespaceScope = "";
+            HI.Type = "struct Point";
+            HI.Definition = "typedef struct Point TPoint;\nstruct Point {}";
+          }},
+      {
+          R"cpp(// Two layers of typedef
         struct Point { int x; int y; };
         typedef struct Point TPoint;
         typedef TPoint TTPoint;
         [[TTP^oint]] tp;
       )cpp",
-      [](HoverInfo &HI) {
-        HI.Name = "TTPoint";
-        HI.Kind = index::SymbolKind::TypeAlias;
-        HI.NamespaceScope = "";
-        HI.Type = "struct Point";
-        HI.Definition = "typedef TPoint TTPoint;\ntypedef struct Point TPoint;\nstruct Point {}";
-      }
-    },
+          [](HoverInfo &HI) {
+            HI.Name = "TTPoint";
+            HI.Kind = index::SymbolKind::TypeAlias;
+            HI.NamespaceScope = "";
+            HI.Type = "struct Point";
+            HI.Definition = "typedef TPoint TTPoint;\ntypedef struct Point "
+                            "TPoint;\nstruct Point {}";
+          }},
   };
   for (const auto &Case : Cases) {
     SCOPED_TRACE(Case.Code);
@@ -4117,7 +4115,8 @@ TEST(Hover, Typedefs) {
 
   ASSERT_TRUE(H && H->Type);
   EXPECT_EQ(H->Type->Type, "int");
-  EXPECT_EQ(H->Definition, "using foo = type<true, int, double>;\nusing type = int");
+  EXPECT_EQ(H->Definition,
+            "using foo = type<true, int, double>;\nusing type = int");
 }
 
 TEST(Hover, EvaluateMacros) {

zyn0217

Thanks for working on this! One thought from me.

zyn0217 · 2024-04-27T04:23:50Z

clang-tools-extra/clangd/Hover.cpp

+
+  QualType Type = TND->getUnderlyingType();
+  while (PrintDeclForType(Type)) {
+    QualType Desugared = Type->getLocallyUnqualifiedSingleStepDesugaredType();


My thought: what do you think of resolving pointer types as well? I could envision cases where pointers are involved in an alias type e.g.

typedef struct _PROCESS_INFORMATION { HANDLE hProcess; HANDLE hThread; DWORD dwProcessId; DWORD dwThreadId; } PROCESS_INFORMATION, *PPROCESS_INFORMATION, *LPPROCESS_INFORMATION;

(excerpted from PROCESS_INFORMATION)
People may want to peek at the definition of the struct while hovering over LPPROCESS_INFORMATION too.

zyn0217 · 2024-04-27T04:51:50Z

(I still need to figure out how to do proper stacked PRs...)

AFAIK, not possible unless our release manager installs e.g. such an application to the repository; but candidly, I have never used it before so I have no idea what kind of shape that would be.

This issue is also being tracked at #56636, in case you don't know.

llvmbot added clang Clang issues not falling into any other category clang-tools-extra clangd clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 22, 2024

[clangd] Show definition of underlying struct when hovering over a ty…

44aba39

…pedef Fixes clangd/clangd#2020

HighCommander4 force-pushed the issue-2020 branch from 457545f to 44aba39 Compare April 25, 2024 22:56

HighCommander4 changed the title ~~[clangd] Show struct members when hovering over a typedef~~ [clangd] Show definition of underlying struct when hovering over a typedef Apr 25, 2024

HighCommander4 requested review from kadircet, hokein and zyn0217 April 25, 2024 22:58

zyn0217 reviewed Apr 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[clangd] Show definition of underlying struct when hovering over a typedef #89570

[clangd] Show definition of underlying struct when hovering over a typedef #89570

HighCommander4 commented Apr 22, 2024

llvmbot commented Apr 22, 2024 •

edited

HighCommander4 commented Apr 22, 2024

HighCommander4 commented Apr 25, 2024

github-actions bot commented Apr 25, 2024

zyn0217 left a comment

zyn0217 Apr 27, 2024

zyn0217 commented Apr 27, 2024

[clangd] Show definition of underlying struct when hovering over a typedef #89570

Are you sure you want to change the base?

[clangd] Show definition of underlying struct when hovering over a typedef #89570

Conversation

HighCommander4 commented Apr 22, 2024

llvmbot commented Apr 22, 2024 • edited

HighCommander4 commented Apr 22, 2024

HighCommander4 commented Apr 25, 2024

github-actions bot commented Apr 25, 2024

zyn0217 left a comment

Choose a reason for hiding this comment

zyn0217 Apr 27, 2024

Choose a reason for hiding this comment

zyn0217 commented Apr 27, 2024

llvmbot commented Apr 22, 2024 •

edited