[PATCH] D60910: [WIP] Dumping the AST to JSON

aaron.ballman closed this revision. aaron.ballman added a comment. Commit in r360622. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60910/new/ https://reviews.llvm.org/D60910 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://l

[PATCH] D60910: [WIP] Dumping the AST to JSON

aaron.ballman added a comment. In D60910#1495673 , @rsmith wrote: > If you're happy with these two conditions, then I have no concerns with this > moving forward: > > - There is no implied stability for the content or format of the dump between > major r

[PATCH] D60910: [WIP] Dumping the AST to JSON

aaron.ballman updated this revision to Diff 199316. aaron.ballman marked 2 inline comments as done. aaron.ballman added a comment. Switched to using the JSON streaming interface rather than a custom solution Updated the test cases for the new formatting CHANGES SINCE LAST ACTION https://review

[PATCH] D60910: [WIP] Dumping the AST to JSON

steveire added inline comments. Comment at: include/clang/AST/DeclBase.h:1139 + void dump(raw_ostream &Out, bool Deserialize = false, +ASTDumpOutputFormat OutputFormat = ADOF_Default) const; aaron.ballman wrote: > steveire wrote: > > I think we've

[PATCH] D60910: [WIP] Dumping the AST to JSON

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. If you're happy with these two conditions, then I have no concerns with this moving forward: - There is no implied stability for the content or format of the dump between major releases, othe

[PATCH] D60910: [WIP] Dumping the AST to JSON

riccibruno marked an inline comment as done. riccibruno added inline comments. Comment at: include/clang/AST/JSONNodeDumper.h:55 + } + + /// Add a child of the current node with an optional label. aaron.ballman wrote: > riccibruno wrote: > > Perhaps you should

[PATCH] D60910: [WIP] Dumping the AST to JSON

aaron.ballman updated this revision to Diff 198477. aaron.ballman marked 14 inline comments as done. aaron.ballman added a comment. Updating based on review feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60910/new/ https://reviews.llvm.org/D60910 Files: include/clang/AST/AST

[PATCH] D60910: [WIP] Dumping the AST to JSON

aaron.ballman added inline comments. Comment at: include/clang/AST/DeclBase.h:1139 + void dump(raw_ostream &Out, bool Deserialize = false, +ASTDumpOutputFormat OutputFormat = ADOF_Default) const; steveire wrote: > I think we've talked about this be

[PATCH] D60910: [WIP] Dumping the AST to JSON

steveire added a comment. Thanks for doing this! I'm glad you were able to do it without needing to change the traverser class. That's a good indicator. I think it would be great if there were somewhere to document the non-stability of the format and content here. Comment at

[PATCH] D60910: [WIP] Dumping the AST to JSON

hintonda added inline comments. Comment at: include/clang/Frontend/FrontendOptions.h:311 + /// Specifies the output format of the AST. + enum ASTOutputFormat { +AOF_Default, aaron.ballman wrote: > hintonda wrote: > > Why can't you use the enum defined in `c

[PATCH] D60910: [WIP] Dumping the AST to JSON

aaron.ballman marked 3 inline comments as done. aaron.ballman added inline comments. Comment at: include/clang/AST/JSONNodeDumper.h:73 + Indentation.clear(); + OS << "\n" << Indentation << "}\n"; + TopLevel = true; hintonda wrote: > Just curious, s

[PATCH] D60910: [WIP] Dumping the AST to JSON

hintonda added a comment. Looks good, but just a couple of questions. Comment at: include/clang/AST/JSONNodeDumper.h:73 + Indentation.clear(); + OS << "\n" << Indentation << "}\n"; + TopLevel = true; Just curious, since you clear Indentation on t

[PATCH] D60910: [WIP] Dumping the AST to JSON

aaron.ballman updated this revision to Diff 196645. aaron.ballman added a comment. Herald added a subscriber: jdoerfert. Rebased to master + small amount of additional functionality. Since it seems this is non-controversial, I will probably commit after next week unless reviewers bring up concer

[PATCH] D60910: [WIP] Dumping the AST to JSON

riccibruno added a comment. In D60910#1473484 , @aaron.ballman wrote: > In D60910#1473441 , @riccibruno > wrote: > > > A few comments/questions: > > > > 1. How stable is the format going to be, and how much state

[PATCH] D60910: [WIP] Dumping the AST to JSON

aaron.ballman marked 2 inline comments as done. aaron.ballman added a comment. In D60910#1473441 , @riccibruno wrote: > A few comments/questions: > > 1. How stable is the format going to be, and how much state is going to be > exposed ? I don't expect i

[PATCH] D60910: [WIP] Dumping the AST to JSON

riccibruno added a subscriber: lebedev.ri. riccibruno added a comment. A few comments/questions: 1. How stable is the format going to be, and how much state is going to be exposed ? 2. I am wondering if it would be possible to separate the logic about how to visit a given AST node, from the log

[PATCH] D60910: [WIP] Dumping the AST to JSON

aaron.ballman created this revision. aaron.ballman added reviewers: rsmith, riccibruno, steveire, dblaikie. Herald added a subscriber: mgorny. This is a work in progress patch that adds the ability to specify an AST dump format on the command line. By default, we continue to dump the AST with its