riccibruno marked 2 inline comments as done. riccibruno added a comment. Thanks for your comments!
In D83183#2132975 <https://reviews.llvm.org/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 &getOS() { 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): <val>`, 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:{{.*}}, col:{{.*}}> 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
