[PATCH] D83183: [clang] Rework how and when APValues are dumped

[Bruno Ricci via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf63e3ea558bb: [clang] Rework how and when APValues are 
dumped (authored by riccibruno).

Changed prior to commit:
  https://reviews.llvm.org/D83183?vs=275575=275633#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83183/new/

https://reviews.llvm.org/D83183

Files:
  clang/include/clang/AST/APValue.h
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/JSONNodeDumper.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/lib/AST/APValue.cpp
  clang/lib/AST/ASTDumper.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/test/AST/alignas_maybe_odr_cleanup.cpp
  clang/test/AST/ast-dump-APValue-anon-union.cpp
  clang/test/AST/ast-dump-APValue-arithmetic.cpp
  clang/test/AST/ast-dump-APValue-array.cpp
  clang/test/AST/ast-dump-APValue-struct.cpp
  clang/test/AST/ast-dump-APValue-todo.cpp
  clang/test/AST/ast-dump-APValue-union.cpp
  clang/test/AST/ast-dump-APValue-vector.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/AST/ast-dump-color.cpp
  clang/test/AST/ast-dump-constant-expr.cpp
  clang/test/AST/ast-dump-decl.cpp
  clang/test/AST/ast-dump-records.cpp
  clang/test/AST/ast-dump-stmt.cpp
  clang/test/AST/pr43983.cpp
  clang/test/Import/switch-stmt/test.cpp
  clang/test/Tooling/clang-check-ast-dump.cpp

Index: clang/test/Tooling/clang-check-ast-dump.cpp
===
--- clang/test/Tooling/clang-check-ast-dump.cpp
+++ clang/test/Tooling/clang-check-ast-dump.cpp
@@ -33,6 +33,7 @@
 // CHECK-ATTR-NEXT: FieldDecl{{.*}}n
 // CHECK-ATTR-NEXT:   AlignedAttr
 // CHECK-ATTR-NEXT: ConstantExpr
+// CHECK-ATTR-NEXT:   value: Int 2
 // CHECK-ATTR-NEXT:   BinaryOperator
 //
 // RUN: clang-check -ast-dump -ast-dump-filter test_namespace::AfterNullNode "%s" -- 2>&1 | FileCheck -check-prefix CHECK-AFTER-NULL %s
Index: clang/test/Import/switch-stmt/test.cpp
===
--- clang/test/Import/switch-stmt/test.cpp
+++ clang/test/Import/switch-stmt/test.cpp
@@ -5,20 +5,26 @@
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 1
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 2
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 3
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 4
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 5
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 5
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 
@@ -30,16 +36,20 @@
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 1
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 2
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 3
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 5
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 
Index: clang/test/AST/pr43983.cpp
===
--- clang/test/AST/pr43983.cpp
+++ clang/test/AST/pr43983.cpp
@@ -9,6 +9,7 @@
 
 struct B { _Alignas(64) struct { int b; };   };
 
-// CHECK: AlignedAttr {{.*}} _Alignas
-// CHECK: ConstantExpr {{.*}} 64
-// CHECK: IntegerLiteral {{.*}} 64
+// CHECK:  | `-AlignedAttr {{.*}}  _Alignas
+// CHECK-NEXT:  |   `-ConstantExpr {{.*}}  'int'
+// CHECK-NEXT:  | |-value: Int 64
+// CHECK-NEXT:  | `-IntegerLiteral {{.*}}  'int' 64
Index: clang/test/AST/ast-dump-stmt.cpp
===
--- clang/test/AST/ast-dump-stmt.cpp
+++ clang/test/AST/ast-dump-stmt.cpp
@@ -130,6 +130,7 @@
 ;
   // CHECK: IfStmt 0x{{[^ ]*}} 
   // CHECK-NEXT: ConstantExpr 0x{{[^ ]*}}  'bool'
+  // CHECK-NEXT: value: Int 1
   // CHECK-NEXT: BinaryOperator
   // CHECK-NEXT: UnaryExprOrTypeTraitExpr
   // CHECK-NEXT: ParenExpr
@@ -144,6 +145,7 @@
 ;
   // CHECK: IfStmt 0x{{[^ ]*}}  has_else
   // CHECK-NEXT: ConstantExpr 0x{{[^ ]*}}  'bool'
+  // CHECK-NEXT: value: Int 1
   // CHECK-NEXT: BinaryOperator
   // CHECK-NEXT: UnaryExprOrTypeTraitExpr
   // CHECK-NEXT: ParenExpr
Index: clang/test/AST/ast-dump-records.cpp
===
--- clang/test/AST/ast-dump-records.cpp
+++ clang/test/AST/ast-dump-records.cpp
@@ -15,7 +15,7 @@
 // CHECK: 

[PATCH] D83183: [clang] Rework how and when APValues are dumped

[Bruno Ricci via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf63e3ea558bb: [clang] Rework how and when APValues are 
dumped (authored by riccibruno).

Changed prior to commit:
  https://reviews.llvm.org/D83183?vs=275783=275828#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83183/new/

https://reviews.llvm.org/D83183

Files:
  clang/include/clang/AST/APValue.h
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/JSONNodeDumper.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/lib/AST/APValue.cpp
  clang/lib/AST/ASTDumper.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/test/AST/alignas_maybe_odr_cleanup.cpp
  clang/test/AST/ast-dump-APValue-anon-union.cpp
  clang/test/AST/ast-dump-APValue-arithmetic.cpp
  clang/test/AST/ast-dump-APValue-array.cpp
  clang/test/AST/ast-dump-APValue-struct.cpp
  clang/test/AST/ast-dump-APValue-todo.cpp
  clang/test/AST/ast-dump-APValue-union.cpp
  clang/test/AST/ast-dump-APValue-vector.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/AST/ast-dump-color.cpp
  clang/test/AST/ast-dump-constant-expr.cpp
  clang/test/AST/ast-dump-decl.cpp
  clang/test/AST/ast-dump-records.cpp
  clang/test/AST/ast-dump-stmt.cpp
  clang/test/AST/pr43983.cpp
  clang/test/Import/switch-stmt/test.cpp
  clang/test/Tooling/clang-check-ast-dump.cpp

Index: clang/test/Tooling/clang-check-ast-dump.cpp
===
--- clang/test/Tooling/clang-check-ast-dump.cpp
+++ clang/test/Tooling/clang-check-ast-dump.cpp
@@ -33,6 +33,7 @@
 // CHECK-ATTR-NEXT: FieldDecl{{.*}}n
 // CHECK-ATTR-NEXT:   AlignedAttr
 // CHECK-ATTR-NEXT: ConstantExpr
+// CHECK-ATTR-NEXT:   value: Int 2
 // CHECK-ATTR-NEXT:   BinaryOperator
 //
 // RUN: clang-check -ast-dump -ast-dump-filter test_namespace::AfterNullNode "%s" -- 2>&1 | FileCheck -check-prefix CHECK-AFTER-NULL %s
Index: clang/test/Import/switch-stmt/test.cpp
===
--- clang/test/Import/switch-stmt/test.cpp
+++ clang/test/Import/switch-stmt/test.cpp
@@ -5,20 +5,26 @@
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 1
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 2
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 3
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 4
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 5
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 5
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 
@@ -30,16 +36,20 @@
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 1
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 2
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 3
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 5
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 
Index: clang/test/AST/pr43983.cpp
===
--- clang/test/AST/pr43983.cpp
+++ clang/test/AST/pr43983.cpp
@@ -9,6 +9,7 @@
 
 struct B { _Alignas(64) struct { int b; };   };
 
-// CHECK: AlignedAttr {{.*}} _Alignas
-// CHECK: ConstantExpr {{.*}} 64
-// CHECK: IntegerLiteral {{.*}} 64
+// CHECK:  | `-AlignedAttr {{.*}}  _Alignas
+// CHECK-NEXT:  |   `-ConstantExpr {{.*}}  'int'
+// CHECK-NEXT:  | |-value: Int 64
+// CHECK-NEXT:  | `-IntegerLiteral {{.*}}  'int' 64
Index: clang/test/AST/ast-dump-stmt.cpp
===
--- clang/test/AST/ast-dump-stmt.cpp
+++ clang/test/AST/ast-dump-stmt.cpp
@@ -130,6 +130,7 @@
 ;
   // CHECK: IfStmt 0x{{[^ ]*}} 
   // CHECK-NEXT: ConstantExpr 0x{{[^ ]*}}  'bool'
+  // CHECK-NEXT: value: Int 1
   // CHECK-NEXT: BinaryOperator
   // CHECK-NEXT: UnaryExprOrTypeTraitExpr
   // CHECK-NEXT: ParenExpr
@@ -144,6 +145,7 @@
 ;
   // CHECK: IfStmt 0x{{[^ ]*}}  has_else
   // CHECK-NEXT: ConstantExpr 0x{{[^ ]*}}  'bool'
+  // CHECK-NEXT: value: Int 1
   // CHECK-NEXT: BinaryOperator
   // CHECK-NEXT: UnaryExprOrTypeTraitExpr
   // CHECK-NEXT: ParenExpr
Index: clang/test/AST/ast-dump-records.cpp
===
--- clang/test/AST/ast-dump-records.cpp
+++ clang/test/AST/ast-dump-records.cpp
@@ -15,7 +15,7 @@
 // CHECK: CXXRecordDecl 0x{{[^ ]*}}  col:8 referenced struct B
 
 struct A {
-  // CHECK: 

[PATCH] D83183: [clang] Rework how and when APValues are dumped

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno marked 2 inline comments as done.
riccibruno added a comment.

Thanks for your comments!

In D83183#2132975 , @aaron.ballman 
wrote:

> Do none of the JSON tests break from this change?


No, but only because I am not modifying the JSON output at all (the JSON output 
is still from `APValue::printPretty`).




Comment at: clang/include/clang/AST/TextNodeDumper.h:163
 
+  raw_ostream () { return OS; }
+

aaron.ballman wrote:
> This is a pretty strange public method; any way to limit its visibility?
I just need it for `dumpAPValueChildren`, but I can make `dumpAPValueChildren` 
a private method of `TextNodeDumper` instead.



Comment at: clang/test/Import/switch-stmt/test.cpp:8
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 1
 // CHECK-NEXT: IntegerLiteral

aaron.ballman wrote:
> I sort of wonder whether we want both the text and the JSON dumpers to dump 
> these as: `value(type): `, as that seems like it produces results that 
> are a bit more well-structured. WDYT?
I'm not sure I follow. The `value` is just a label for the child of the 
`VarDecl`.
If you look at a more complex example such as:
```
VarDecl {{.*}}  col:{{.*}} s4 'const S4'
|-value: Struct
| |-base: Struct
| | `-fields: Int 0, Union .j Int 0
| |-fields: Int 1, Int 2, Int 3
| |-field: Struct
| `-fields: Int 4, Int 5, Int 6
```

There is no other `value` label.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83183/new/

https://reviews.llvm.org/D83183



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83183: [clang] Rework how and when APValues are dumped

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 275713.
riccibruno added a comment.

Format the tests and make them more relevant.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83183/new/

https://reviews.llvm.org/D83183

Files:
  clang/include/clang/AST/APValue.h
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/JSONNodeDumper.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/lib/AST/APValue.cpp
  clang/lib/AST/ASTDumper.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/test/AST/alignas_maybe_odr_cleanup.cpp
  clang/test/AST/ast-dump-APValue-anon-union.cpp
  clang/test/AST/ast-dump-APValue-arithmetic.cpp
  clang/test/AST/ast-dump-APValue-array.cpp
  clang/test/AST/ast-dump-APValue-struct.cpp
  clang/test/AST/ast-dump-APValue-todo.cpp
  clang/test/AST/ast-dump-APValue-union.cpp
  clang/test/AST/ast-dump-APValue-vector.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/AST/ast-dump-color.cpp
  clang/test/AST/ast-dump-constant-expr.cpp
  clang/test/AST/ast-dump-decl.cpp
  clang/test/AST/ast-dump-records.cpp
  clang/test/AST/ast-dump-stmt.cpp
  clang/test/AST/pr43983.cpp
  clang/test/Import/switch-stmt/test.cpp
  clang/test/Tooling/clang-check-ast-dump.cpp

Index: clang/test/Tooling/clang-check-ast-dump.cpp
===
--- clang/test/Tooling/clang-check-ast-dump.cpp
+++ clang/test/Tooling/clang-check-ast-dump.cpp
@@ -33,6 +33,7 @@
 // CHECK-ATTR-NEXT: FieldDecl{{.*}}n
 // CHECK-ATTR-NEXT:   AlignedAttr
 // CHECK-ATTR-NEXT: ConstantExpr
+// CHECK-ATTR-NEXT:   value: Int 2
 // CHECK-ATTR-NEXT:   BinaryOperator
 //
 // RUN: clang-check -ast-dump -ast-dump-filter test_namespace::AfterNullNode "%s" -- 2>&1 | FileCheck -check-prefix CHECK-AFTER-NULL %s
Index: clang/test/Import/switch-stmt/test.cpp
===
--- clang/test/Import/switch-stmt/test.cpp
+++ clang/test/Import/switch-stmt/test.cpp
@@ -5,20 +5,26 @@
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 1
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 2
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 3
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 4
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 5
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 5
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 
@@ -30,16 +36,20 @@
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 1
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 2
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 3
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 5
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 
Index: clang/test/AST/pr43983.cpp
===
--- clang/test/AST/pr43983.cpp
+++ clang/test/AST/pr43983.cpp
@@ -9,6 +9,7 @@
 
 struct B { _Alignas(64) struct { int b; };   };
 
-// CHECK: AlignedAttr {{.*}} _Alignas
-// CHECK: ConstantExpr {{.*}} 64
-// CHECK: IntegerLiteral {{.*}} 64
+// CHECK:  | `-AlignedAttr {{.*}}  _Alignas
+// CHECK-NEXT:  |   `-ConstantExpr {{.*}}  'int'
+// CHECK-NEXT:  | |-value: Int 64
+// CHECK-NEXT:  | `-IntegerLiteral {{.*}}  'int' 64
Index: clang/test/AST/ast-dump-stmt.cpp
===
--- clang/test/AST/ast-dump-stmt.cpp
+++ clang/test/AST/ast-dump-stmt.cpp
@@ -130,6 +130,7 @@
 ;
   // CHECK: IfStmt 0x{{[^ ]*}} 
   // CHECK-NEXT: ConstantExpr 0x{{[^ ]*}}  'bool'
+  // CHECK-NEXT: value: Int 1
   // CHECK-NEXT: BinaryOperator
   // CHECK-NEXT: UnaryExprOrTypeTraitExpr
   // CHECK-NEXT: ParenExpr
@@ -144,6 +145,7 @@
 ;
   // CHECK: IfStmt 0x{{[^ ]*}}  has_else
   // CHECK-NEXT: ConstantExpr 0x{{[^ ]*}}  'bool'
+  // CHECK-NEXT: value: Int 1
   // CHECK-NEXT: BinaryOperator
   // CHECK-NEXT: UnaryExprOrTypeTraitExpr
   // CHECK-NEXT: ParenExpr
Index: clang/test/AST/ast-dump-records.cpp
===
--- clang/test/AST/ast-dump-records.cpp
+++ clang/test/AST/ast-dump-records.cpp
@@ -15,7 +15,7 @@
 // CHECK: CXXRecordDecl 0x{{[^ ]*}}  col:8 referenced struct B
 
 struct A {
-  // CHECK: CXXRecordDecl 0x{{[^ ]*}} prev 0x{{[^ ]*}}  line:[[@LINE-1]]:8 struct A definition
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}} prev 0x{{[^ ]*}}  

[PATCH] D83183: [clang] Rework how and when APValues are dumped

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Do none of the JSON tests break from this change?




Comment at: clang/include/clang/AST/TextNodeDumper.h:163
 
+  raw_ostream () { return OS; }
+

This is a pretty strange public method; any way to limit its visibility?



Comment at: clang/test/Import/switch-stmt/test.cpp:8
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 1
 // CHECK-NEXT: IntegerLiteral

I sort of wonder whether we want both the text and the JSON dumpers to dump 
these as: `value(type): `, as that seems like it produces results that are 
a bit more well-structured. WDYT?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83183/new/

https://reviews.llvm.org/D83183



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83183: [clang] Rework how and when APValues are dumped

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!




Comment at: clang/test/Import/switch-stmt/test.cpp:8
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 1
 // CHECK-NEXT: IntegerLiteral

riccibruno wrote:
> aaron.ballman wrote:
> > I sort of wonder whether we want both the text and the JSON dumpers to dump 
> > these as: `value(type): `, as that seems like it produces results that 
> > are a bit more well-structured. WDYT?
> I'm not sure I follow. The `value` is just a label for the child of the 
> `VarDecl`.
> If you look at a more complex example such as:
> ```
> VarDecl {{.*}}  col:{{.*}} s4 'const S4'
> |-value: Struct
> | |-base: Struct
> | | `-fields: Int 0, Union .j Int 0
> | |-fields: Int 1, Int 2, Int 3
> | |-field: Struct
> | `-fields: Int 4, Int 5, Int 6
> ```
> 
> There is no other `value` label.
Ah, I was misunderstanding the ouput.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83183/new/

https://reviews.llvm.org/D83183



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83183: [clang] Rework how and when APValues are dumped

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 275783.
riccibruno added a comment.

Make `dumpAPValueChildren` a private member function and remove 
`TextNodeDumper::getOS`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83183/new/

https://reviews.llvm.org/D83183

Files:
  clang/include/clang/AST/APValue.h
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/JSONNodeDumper.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/lib/AST/APValue.cpp
  clang/lib/AST/ASTDumper.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/test/AST/alignas_maybe_odr_cleanup.cpp
  clang/test/AST/ast-dump-APValue-anon-union.cpp
  clang/test/AST/ast-dump-APValue-arithmetic.cpp
  clang/test/AST/ast-dump-APValue-array.cpp
  clang/test/AST/ast-dump-APValue-struct.cpp
  clang/test/AST/ast-dump-APValue-todo.cpp
  clang/test/AST/ast-dump-APValue-union.cpp
  clang/test/AST/ast-dump-APValue-vector.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/AST/ast-dump-color.cpp
  clang/test/AST/ast-dump-constant-expr.cpp
  clang/test/AST/ast-dump-decl.cpp
  clang/test/AST/ast-dump-records.cpp
  clang/test/AST/ast-dump-stmt.cpp
  clang/test/AST/pr43983.cpp
  clang/test/Import/switch-stmt/test.cpp
  clang/test/Tooling/clang-check-ast-dump.cpp

Index: clang/test/Tooling/clang-check-ast-dump.cpp
===
--- clang/test/Tooling/clang-check-ast-dump.cpp
+++ clang/test/Tooling/clang-check-ast-dump.cpp
@@ -33,6 +33,7 @@
 // CHECK-ATTR-NEXT: FieldDecl{{.*}}n
 // CHECK-ATTR-NEXT:   AlignedAttr
 // CHECK-ATTR-NEXT: ConstantExpr
+// CHECK-ATTR-NEXT:   value: Int 2
 // CHECK-ATTR-NEXT:   BinaryOperator
 //
 // RUN: clang-check -ast-dump -ast-dump-filter test_namespace::AfterNullNode "%s" -- 2>&1 | FileCheck -check-prefix CHECK-AFTER-NULL %s
Index: clang/test/Import/switch-stmt/test.cpp
===
--- clang/test/Import/switch-stmt/test.cpp
+++ clang/test/Import/switch-stmt/test.cpp
@@ -5,20 +5,26 @@
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 1
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 2
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 3
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 4
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 5
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 5
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 
@@ -30,16 +36,20 @@
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 1
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 2
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 3
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 5
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 
Index: clang/test/AST/pr43983.cpp
===
--- clang/test/AST/pr43983.cpp
+++ clang/test/AST/pr43983.cpp
@@ -9,6 +9,7 @@
 
 struct B { _Alignas(64) struct { int b; };   };
 
-// CHECK: AlignedAttr {{.*}} _Alignas
-// CHECK: ConstantExpr {{.*}} 64
-// CHECK: IntegerLiteral {{.*}} 64
+// CHECK:  | `-AlignedAttr {{.*}}  _Alignas
+// CHECK-NEXT:  |   `-ConstantExpr {{.*}}  'int'
+// CHECK-NEXT:  | |-value: Int 64
+// CHECK-NEXT:  | `-IntegerLiteral {{.*}}  'int' 64
Index: clang/test/AST/ast-dump-stmt.cpp
===
--- clang/test/AST/ast-dump-stmt.cpp
+++ clang/test/AST/ast-dump-stmt.cpp
@@ -130,6 +130,7 @@
 ;
   // CHECK: IfStmt 0x{{[^ ]*}} 
   // CHECK-NEXT: ConstantExpr 0x{{[^ ]*}}  'bool'
+  // CHECK-NEXT: value: Int 1
   // CHECK-NEXT: BinaryOperator
   // CHECK-NEXT: UnaryExprOrTypeTraitExpr
   // CHECK-NEXT: ParenExpr
@@ -144,6 +145,7 @@
 ;
   // CHECK: IfStmt 0x{{[^ ]*}}  has_else
   // CHECK-NEXT: ConstantExpr 0x{{[^ ]*}}  'bool'
+  // CHECK-NEXT: value: Int 1
   // CHECK-NEXT: BinaryOperator
   // CHECK-NEXT: UnaryExprOrTypeTraitExpr
   // CHECK-NEXT: ParenExpr
Index: clang/test/AST/ast-dump-records.cpp
===
--- clang/test/AST/ast-dump-records.cpp
+++ clang/test/AST/ast-dump-records.cpp
@@ -15,7 +15,7 @@
 // CHECK: CXXRecordDecl 0x{{[^ ]*}}  col:8 referenced struct B
 
 struct A {
-  // CHECK: CXXRecordDecl 0x{{[^ ]*}} prev 0x{{[^ ]*}}  line:[[@LINE-1]]:8 struct A definition
+  // CHECK: 

[PATCH] D83183: [clang] Rework how and when APValues are dumped

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno created this revision.
riccibruno added reviewers: aaron.ballman, steveire, ilya-biryukov, sammccall, 
martong.
riccibruno added a project: clang.
Herald added subscribers: cfe-commits, rnkovacs.

Currently `APValue`s are dumped as a single string. This becomes quickly 
completely
unreadable since `APValue` is a tree-like structure. Even a simple example is 
not pretty:

  struct S { int arr[4]; float f; };
  constexpr S s = { .arr = {1,2}, .f = 3.1415f };
  // Struct  fields: Array: Int: 1, Int: 2, 2 x Int: 0, Float: 3.141500e+00

With this patch this becomes:

  -Struct
   |-field: Array size=4
   | |-elements: Int 1, Int 2
   | `-filler: 2 x Int 0
   `-field: Float 3.141500e+00

Additionally `APValue`s are currently only dumped as part of visiting a 
`ConstantExpr`.
This patch also dump the value of the initializer (if any) of constexpr 
variable declarations:

  constexpr int foo(int a, int b) { return a + b - 42; }
  constexpr int a = 1, b = 2;
  constexpr int c = foo(a, b) > 0 ? foo(a, b) : foo(b, a);
  // VarDecl 0x6218aec8  col:17 c 'const int' constexpr cinit
  // |-value: Int -39
  // `-ConditionalOperator 0x6218b4d0  'int'
  // 

Do the above by moving the dump functions to `TextNodeDumper` which already has
the machinery to display trees. The cases `APValue::LValue`, 
`APValue::MemberPointer`
and `APValue::AddrLabelDiff` are left as they were before (unimplemented).

We try to display multiple elements on the same line if they are considered to 
be "simple".
This is to avoid wasting large amounts of vertical space in an example like:

  constexpr int arr[8] = {0,1,2,3,4,5,6,7};
  // VarDecl 0x6218bb78  col:17 arr 'int const[8]' constexpr 
cinit
  // |-value: Array size=8
  // | |-elements: Int 0, Int 1, Int 2, Int 3
  // | `-elements: Int 4, Int 5, Int 6, Int 7


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83183

Files:
  clang/include/clang/AST/APValue.h
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/JSONNodeDumper.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/lib/AST/APValue.cpp
  clang/lib/AST/ASTDumper.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/test/AST/alignas_maybe_odr_cleanup.cpp
  clang/test/AST/ast-dump-APValue-anon-union.cpp
  clang/test/AST/ast-dump-APValue-arithmetic.cpp
  clang/test/AST/ast-dump-APValue-array.cpp
  clang/test/AST/ast-dump-APValue-struct.cpp
  clang/test/AST/ast-dump-APValue-todo.cpp
  clang/test/AST/ast-dump-APValue-union.cpp
  clang/test/AST/ast-dump-APValue-vector.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/AST/ast-dump-color.cpp
  clang/test/AST/ast-dump-constant-expr.cpp
  clang/test/AST/ast-dump-decl.cpp
  clang/test/AST/ast-dump-records.cpp
  clang/test/AST/ast-dump-stmt.cpp
  clang/test/AST/pr43983.cpp
  clang/test/Import/switch-stmt/test.cpp
  clang/test/Tooling/clang-check-ast-dump.cpp

Index: clang/test/Tooling/clang-check-ast-dump.cpp
===
--- clang/test/Tooling/clang-check-ast-dump.cpp
+++ clang/test/Tooling/clang-check-ast-dump.cpp
@@ -33,6 +33,7 @@
 // CHECK-ATTR-NEXT: FieldDecl{{.*}}n
 // CHECK-ATTR-NEXT:   AlignedAttr
 // CHECK-ATTR-NEXT: ConstantExpr
+// CHECK-ATTR-NEXT:   value: Int 2
 // CHECK-ATTR-NEXT:   BinaryOperator
 //
 // RUN: clang-check -ast-dump -ast-dump-filter test_namespace::AfterNullNode "%s" -- 2>&1 | FileCheck -check-prefix CHECK-AFTER-NULL %s
Index: clang/test/Import/switch-stmt/test.cpp
===
--- clang/test/Import/switch-stmt/test.cpp
+++ clang/test/Import/switch-stmt/test.cpp
@@ -5,20 +5,26 @@
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 1
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 2
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 3
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 4
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 5
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 5
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 
@@ -30,16 +36,20 @@
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 1
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 2
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 3
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 5
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 
Index: