Skip to content

[Clang] Fix AST dump for {CXXDefaultArgExpr, CXXDefaultInitExpr} #88269

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 12, 2024

Conversation

yronglin
Copy link
Contributor

@yronglin yronglin commented Apr 10, 2024

This PR fix a AST dump issue since #80001

When Clang dumps CXXDefaultArgExpr/CXXDefaultInitExpr, there has no recursively dump the complete CXXDefaultArgExpr/CXXDefaultInitExpr.

Since this PR, Clang will recursively dump a CXXDefaultArgExpr/CXXDefaultInitExpr node, even if the node has no rewritten init.

Consider:

struct A {
  int arr[1];
};

struct B {
  const A &a = A{{0}};
};

void test() {
  B b{};
}

Before:

`-FunctionDecl <line:9:1, line:11:1> line:9:6 test 'void ()'
  `-CompoundStmt <col:13, line:11:1>
    `-DeclStmt <line:10:3, col:8>
      `-VarDecl <col:3, col:7> col:5 b 'B' listinit
        `-InitListExpr <col:6, col:7> 'B'
          `-CXXDefaultInitExpr <col:7> 'const A' lvalue has rewritten init
            `-ExprWithCleanups <line:6:16, col:21> 'const A' lvalue

After:

`-FunctionDecl 0x15a9455a8 <line:9:1, line:11:1> line:9:6 test 'void ()'
  `-CompoundStmt 0x15a945850 <col:13, line:11:1>
    `-DeclStmt 0x15a945838 <line:10:3, col:8>
      `-VarDecl 0x15a945708 <col:3, col:7> col:5 b 'B' listinit
        `-InitListExpr 0x15a9457b0 <col:6, col:7> 'B'
          `-CXXDefaultInitExpr 0x15a9457f8 <col:7> 'const A' lvalue has rewritten init
            `-ExprWithCleanups 0x15a945568 <line:6:16, col:21> 'const A' lvalue
              `-MaterializeTemporaryExpr 0x15a945500 <col:16, col:21> 'const A' lvalue extended by Field 0x15a945160 'a' 'const A &'
                `-ImplicitCastExpr 0x15a9454e8 <col:16, col:21> 'const A' <NoOp>
                  `-CXXFunctionalCastExpr 0x15a9454c0 <col:16, col:21> 'A' functional cast to A <NoOp>
                    `-InitListExpr 0x15a9452c0 <col:17, col:21> 'A'
                      `-InitListExpr 0x15a945308 <col:18, col:20> 'int[1]'
                        `-IntegerLiteral 0x15a945210 <col:19> 'int' 0

@yronglin yronglin requested a review from sam-mccall April 10, 2024 13:49
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 10, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 10, 2024

@llvm/pr-subscribers-clang

Author: None (yronglin)

Changes

This PR fix a AST dump issue since #80001

When Clang dumps CXXDefaultArgExpr/CXXDefaultInitExpr, there has no recursively dump the complete CXXDefaultArgExpr/CXXDefaultInitExpr.

Since this PR, Clang will recursively dump a CXXDefaultArgExpr/CXXDefaultInitExpr node, even if the node has no rewritten init.

Consider:

Before:

`-FunctionDecl &lt;line:9:1, line:11:1&gt; line:9:6 test 'void ()'
  `-CompoundStmt &lt;col:13, line:11:1&gt;
    `-DeclStmt &lt;line:10:3, col:8&gt;
      `-VarDecl &lt;col:3, col:7&gt; col:5 b 'B' listinit
        `-InitListExpr &lt;col:6, col:7&gt; 'B'
          `-CXXDefaultInitExpr &lt;col:7&gt; 'const A' lvalue has rewritten init
            `-ExprWithCleanups &lt;line:6:16, col:21&gt; 'const A' lvalue

After:

`-FunctionDecl 0x15a9455a8 &lt;line:9:1, line:11:1&gt; line:9:6 test 'void ()'
  `-CompoundStmt 0x15a945850 &lt;col:13, line:11:1&gt;
    `-DeclStmt 0x15a945838 &lt;line:10:3, col:8&gt;
      `-VarDecl 0x15a945708 &lt;col:3, col:7&gt; col:5 b 'B' listinit
        `-InitListExpr 0x15a9457b0 &lt;col:6, col:7&gt; 'B'
          `-CXXDefaultInitExpr 0x15a9457f8 &lt;col:7&gt; 'const A' lvalue has rewritten init
            `-ExprWithCleanups 0x15a945568 &lt;line:6:16, col:21&gt; 'const A' lvalue
              `-MaterializeTemporaryExpr 0x15a945500 &lt;col:16, col:21&gt; 'const A' lvalue extended by Field 0x15a945160 'a' 'const A &amp;'
                `-ImplicitCastExpr 0x15a9454e8 &lt;col:16, col:21&gt; 'const A' &lt;NoOp&gt;
                  `-CXXFunctionalCastExpr 0x15a9454c0 &lt;col:16, col:21&gt; 'A' functional cast to A &lt;NoOp&gt;
                    `-InitListExpr 0x15a9452c0 &lt;col:17, col:21&gt; 'A'
                      `-InitListExpr 0x15a945308 &lt;col:18, col:20&gt; 'int[1]'
                        `-IntegerLiteral 0x15a945210 &lt;col:19&gt; 'int' 0

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

5 Files Affected:

  • (modified) clang/include/clang/AST/ASTNodeTraverser.h (+8)
  • (modified) clang/lib/AST/TextNodeDumper.cpp (+2-12)
  • (added) clang/test/AST/ast-dump-default-init.cpp (+24)
  • (modified) clang/test/AST/ast-dump-for-range-lifetime.cpp (+21)
  • (modified) clang/unittests/AST/ASTTraverserTest.cpp (+8)
diff --git a/clang/include/clang/AST/ASTNodeTraverser.h b/clang/include/clang/AST/ASTNodeTraverser.h
index 94e7dd817809dd..a6e3b05b005968 100644
--- a/clang/include/clang/AST/ASTNodeTraverser.h
+++ b/clang/include/clang/AST/ASTNodeTraverser.h
@@ -932,6 +932,14 @@ class ASTNodeTraverser
       Visit(TArg);
   }
 
+  void VisitCXXDefaultArgExpr(const CXXDefaultArgExpr *Node) {
+    Visit(Node->getExpr());
+  }
+
+  void VisitCXXDefaultInitExpr(const CXXDefaultInitExpr *Node) {
+    Visit(Node->getExpr());
+  }
+
   // Implements Visit methods for Attrs.
 #include "clang/AST/AttrNodeTraverse.inc"
 };
diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp
index 431f5d8bdb2b5f..830d3ff0faebd4 100644
--- a/clang/lib/AST/TextNodeDumper.cpp
+++ b/clang/lib/AST/TextNodeDumper.cpp
@@ -1439,23 +1439,13 @@ void TextNodeDumper::VisitExpressionTraitExpr(const ExpressionTraitExpr *Node) {
 }
 
 void TextNodeDumper::VisitCXXDefaultArgExpr(const CXXDefaultArgExpr *Node) {
-  if (Node->hasRewrittenInit()) {
+  if (Node->hasRewrittenInit())
     OS << " has rewritten init";
-    AddChild([=] {
-      ColorScope Color(OS, ShowColors, StmtColor);
-      Visit(Node->getExpr());
-    });
-  }
 }
 
 void TextNodeDumper::VisitCXXDefaultInitExpr(const CXXDefaultInitExpr *Node) {
-  if (Node->hasRewrittenInit()) {
+  if (Node->hasRewrittenInit())
     OS << " has rewritten init";
-    AddChild([=] {
-      ColorScope Color(OS, ShowColors, StmtColor);
-      Visit(Node->getExpr());
-    });
-  }
 }
 
 void TextNodeDumper::VisitMaterializeTemporaryExpr(
diff --git a/clang/test/AST/ast-dump-default-init.cpp b/clang/test/AST/ast-dump-default-init.cpp
new file mode 100644
index 00000000000000..a2ce4ba57aa968
--- /dev/null
+++ b/clang/test/AST/ast-dump-default-init.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -ast-dump %s | FileCheck %s
+
+// CXXDefaultArgExpr should inherit dependence from the inner Expr, in this case
+// RecoveryExpr.
+
+struct A {
+  int arr[1];
+};
+
+struct B {
+  const A &a = A{{0}};
+};
+
+void test() {
+  B b{};
+}
+// CHECK: -CXXDefaultInitExpr 0x{{[^ ]*}} <{{.*}}> 'const A' lvalue has rewritten init
+// CHECK-NEXT:  `-ExprWithCleanups 0x{{[^ ]*}} <{{.*}}> 'const A' lvalue
+// CHECK-NEXT:    `-MaterializeTemporaryExpr 0x{{[^ ]*}} <{{.*}}> 'const A' lvalue extended by Var 0x{{[^ ]*}} 'b' 'B'
+// CHECK-NEXT:      `-ImplicitCastExpr 0x{{[^ ]*}} <{{.*}}> 'const A' <NoOp>
+// CHECK-NEXT:        `-CXXFunctionalCastExpr 0x{{[^ ]*}} <{{.*}}> 'A' functional cast to A <NoOp>
+// CHECK-NEXT:          `-InitListExpr 0x{{[^ ]*}} <{{.*}}> 'A'
+// CHECK-NEXT:            `-InitListExpr 0x{{[^ ]*}} <{{.*}}> 'int[1]'
+// CHECK-NEXT:              `-IntegerLiteral 0x{{[^ ]*}} <{{.*}}> 'int' 0
diff --git a/clang/test/AST/ast-dump-for-range-lifetime.cpp b/clang/test/AST/ast-dump-for-range-lifetime.cpp
index 88b838268be2e0..0e92b6990ed504 100644
--- a/clang/test/AST/ast-dump-for-range-lifetime.cpp
+++ b/clang/test/AST/ast-dump-for-range-lifetime.cpp
@@ -132,6 +132,9 @@ void test4() {
   // CHECK-NEXT:  |       | `-DeclRefExpr {{.*}} 'int (&(const A &))[3]' lvalue Function {{.*}} 'default_arg_fn' 'int (&(const A &))[3]'
   // CHECK-NEXT:  |       `-CXXDefaultArgExpr {{.*}} <<invalid sloc>> 'const A':'const P2718R0::A' lvalue has rewritten init
   // CHECK-NEXT:  |         `-MaterializeTemporaryExpr {{.*}} 'const A':'const P2718R0::A' lvalue extended by Var {{.*}} '__range1' 'int (&)[3]'
+  // CHECK-NEXT:  |           `-ImplicitCastExpr {{.*}} 'const A':'const P2718R0::A' <NoOp>
+  // CHECK-NEXT:  |             `-CXXBindTemporaryExpr {{.*}} 'A':'P2718R0::A' (CXXTemporary {{.*}})
+  // CHECK-NEXT:  |               `-CXXTemporaryObjectExpr {{.*}} 'A':'P2718R0::A' 'void ()'
   for (auto e : default_arg_fn()) 
     bar(e);
 }
@@ -179,10 +182,19 @@ void test5() {
   // CHECK-NEXT:  |               |       |       |     `-CXXTemporaryObjectExpr {{.*}} 'A':'P2718R0::A' 'void ()'
   // CHECK-NEXT:  |               |       |       `-CXXDefaultArgExpr {{.*}} <<invalid sloc>> 'const DefaultA':'const P2718R0::DefaultA' lvalue has rewritten init
   // CHECK-NEXT:  |               |       |         `-MaterializeTemporaryExpr {{.*}} 'const DefaultA':'const P2718R0::DefaultA' lvalue extended by Var {{.*}} '__range1' 'int (&)[3]'
+  // CHECK-NEXT:  |               |       |           `-ImplicitCastExpr {{.*}} 'const DefaultA':'const P2718R0::DefaultA' <NoOp>
+  // CHECK-NEXT:  |               |       |             `-CXXBindTemporaryExpr {{.*}} 'DefaultA':'P2718R0::DefaultA' (CXXTemporary {{.*}})
+  // CHECK-NEXT:  |               |       |               `-CXXTemporaryObjectExpr {{.*}} 'DefaultA':'P2718R0::DefaultA' 'void ()'
   // CHECK-NEXT:  |               |       `-CXXDefaultArgExpr {{.*}} <<invalid sloc>> 'const DefaultA':'const P2718R0::DefaultA' lvalue has rewritten init
   // CHECK-NEXT:  |               |         `-MaterializeTemporaryExpr {{.*}} 'const DefaultA':'const P2718R0::DefaultA' lvalue extended by Var {{.*}} '__range1' 'int (&)[3]'
+  // CHECK-NEXT:  |               |           `-ImplicitCastExpr {{.*}} 'const DefaultA':'const P2718R0::DefaultA' <NoOp>
+  // CHECK-NEXT:  |               |             `-CXXBindTemporaryExpr {{.*}} 'DefaultA':'P2718R0::DefaultA' (CXXTemporary {{.*}})
+  // CHECK-NEXT:  |               |               `-CXXTemporaryObjectExpr {{.*}} 'DefaultA':'P2718R0::DefaultA' 'void ()'
   // CHECK-NEXT:  |               `-CXXDefaultArgExpr {{.*}} <<invalid sloc>> 'const DefaultA':'const P2718R0::DefaultA' lvalue has rewritten init
   // CHECK-NEXT:  |                 `-MaterializeTemporaryExpr {{.*}} 'const DefaultA':'const P2718R0::DefaultA' lvalue extended by Var {{.*}} '__range1' 'int (&)[3]'
+  // CHECK-NEXT:  |                   `-ImplicitCastExpr {{.*}} 'const DefaultA':'const P2718R0::DefaultA' <NoOp>
+  // CHECK-NEXT:  |                     `-CXXBindTemporaryExpr {{.*}} 'DefaultA':'P2718R0::DefaultA' (CXXTemporary {{.*}})
+  // CHECK-NEXT:  |                       `-CXXTemporaryObjectExpr {{.*}} 'DefaultA':'P2718R0::DefaultA' 'void ()'
   for (auto e : default_arg_fn(foo(foo(foo(A())))))
     bar(e);
 }
@@ -219,10 +231,19 @@ void test6() {
   // CHECK-NEXT:  |           |       |       |     `-CXXTemporaryObjectExpr {{.*}} 'C':'P2718R0::C' 'void ()'
   // CHECK-NEXT:  |           |       |       `-CXXDefaultArgExpr {{.*}} <<invalid sloc>> 'const DefaultA':'const P2718R0::DefaultA' lvalue has rewritten init
   // CHECK-NEXT:  |           |       |         `-MaterializeTemporaryExpr {{.*}} 'const DefaultA':'const P2718R0::DefaultA' lvalue extended by Var {{.*}} '__range1' 'C &&'
+  // CHECK-NEXT:  |           |       |           `-ImplicitCastExpr {{.*}} 'const DefaultA':'const P2718R0::DefaultA' <NoOp>
+  // CHECK-NEXT:  |           |       |             `-CXXBindTemporaryExpr {{.*}} 'DefaultA':'P2718R0::DefaultA' (CXXTemporary {{.*}})
+  // CHECK-NEXT:  |           |       |               `-CXXTemporaryObjectExpr {{.*}} 'DefaultA':'P2718R0::DefaultA' 'void ()'
   // CHECK-NEXT:  |           |       `-CXXDefaultArgExpr {{.*}} <<invalid sloc>> 'const DefaultA':'const P2718R0::DefaultA' lvalue has rewritten init
   // CHECK-NEXT:  |           |         `-MaterializeTemporaryExpr {{.*}} 'const DefaultA':'const P2718R0::DefaultA' lvalue extended by Var {{.*}} '__range1' 'C &&'
+  // CHECK-NEXT:  |           |           `-ImplicitCastExpr {{.*}} 'const DefaultA':'const P2718R0::DefaultA' <NoOp>
+  // CHECK-NEXT:  |           |             `-CXXBindTemporaryExpr {{.*}} 'DefaultA':'P2718R0::DefaultA' (CXXTemporary {{.*}})
+  // CHECK-NEXT:  |           |               `-CXXTemporaryObjectExpr {{.*}} 'DefaultA':'P2718R0::DefaultA' 'void ()'
   // CHECK-NEXT:  |           `-CXXDefaultArgExpr {{.*}} <<invalid sloc>> 'const DefaultA':'const P2718R0::DefaultA' lvalue has rewritten init
   // CHECK-NEXT:  |             `-MaterializeTemporaryExpr {{.*}} 'const DefaultA':'const P2718R0::DefaultA' lvalue extended by Var {{.*}} '__range1' 'C &&'
+  // CHECK-NEXT:  |               `-ImplicitCastExpr {{.*}} 'const DefaultA':'const P2718R0::DefaultA' <NoOp>
+  // CHECK-NEXT:  |                 `-CXXBindTemporaryExpr {{.*}} 'DefaultA':'P2718R0::DefaultA' (CXXTemporary {{.*}})
+  // CHECK-NEXT:  |                   `-CXXTemporaryObjectExpr {{.*}} 'DefaultA':'P2718R0::DefaultA' 'void ()'
   for (auto e : C(0, C(0, C(0, C()))))
     bar(e);
 }
diff --git a/clang/unittests/AST/ASTTraverserTest.cpp b/clang/unittests/AST/ASTTraverserTest.cpp
index 362f37655a0ae4..8b6e3e90c0ea67 100644
--- a/clang/unittests/AST/ASTTraverserTest.cpp
+++ b/clang/unittests/AST/ASTTraverserTest.cpp
@@ -368,6 +368,8 @@ FunctionDecl 'stringConstruct'
   |             |-ImplicitCastExpr
   |             | `-StringLiteral
   |             `-CXXDefaultArgExpr
+  |               `-UnaryOperator
+  |                 `-IntegerLiteral
   `-ExprWithCleanups
     `-CXXOperatorCallExpr
       |-ImplicitCastExpr
@@ -378,6 +380,8 @@ FunctionDecl 'stringConstruct'
           |-ImplicitCastExpr
           | `-StringLiteral
           `-CXXDefaultArgExpr
+            `-UnaryOperator
+              `-IntegerLiteral
 )cpp");
 
     EXPECT_EQ(dumpASTString(TK_IgnoreUnlessSpelledInSource,
@@ -415,6 +419,8 @@ FunctionDecl 'overloadCall'
   |             |-ImplicitCastExpr
   |             | `-StringLiteral
   |             `-CXXDefaultArgExpr
+  |               `-UnaryOperator
+  |                 `-IntegerLiteral
   `-CXXMemberCallExpr
     `-MemberExpr
       `-ParenExpr
@@ -1219,6 +1225,7 @@ CXXRecordDecl 'Record'
 | | `-IntegerLiteral
 | |-CXXCtorInitializer 'm_i2'
 | | `-CXXDefaultInitExpr
+| |   `-IntegerLiteral
 | |-CXXCtorInitializer 'm_s'
 | | `-CXXConstructExpr
 | `-CompoundStmt
@@ -1485,6 +1492,7 @@ CallExpr
 | `-DeclRefExpr 'hasDefaultArg'
 |-IntegerLiteral
 `-CXXDefaultArgExpr
+  `-IntegerLiteral
 )cpp");
     EXPECT_EQ(dumpASTString(TK_IgnoreUnlessSpelledInSource,
                             BN[0].getNodeAs<CallExpr>("funcCall")),

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but you should check JSONNodeDumper.cpp as well to see if it needs similar changes.

@yronglin
Copy link
Contributor Author

LGTM but you should check JSONNodeDumper.cpp as well to see if it needs similar changes.

Wow! I've missed that before, I have now added support for JSONNodeDumper.cpp. Many thanks for your review!

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM again, thanks for the extra work to get JSON dumping as well!

@yronglin yronglin merged commit 986d0db into llvm:main Apr 12, 2024
@yronglin
Copy link
Contributor Author

LGTM again, thanks for the extra work to get JSON dumping as well!

Thanks for your review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants