riccibruno added a comment. In D60910#1473484 <https://reviews.llvm.org/D60910#1473484>, @aaron.ballman wrote:
> In D60910#1473441 <https://reviews.llvm.org/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 it to be particularly stable (I'm not suggesting we conform to > some particular schema) and for my own needs, I'm happy enough if new Clang > releases break us due to new, deleted, or renamed information. I need to > expose as much state as required for our research team to import the data and > reason about the AST; I'd guess it's going to be roughly the same as the > textual dumper, but different in spots. I am fine with that. > > >> 2. I am wondering if it would be possible to separate the logic about how to >> visit a given AST node, from the logic about what to do with the node's >> state. For example it seems to me that `JSONNodeDumper::VisitVarDecl` and >> `TextNodeDumper::VisitVarDecl` are somewhat similar. You could instead >> imagine having a generic way to visit each node, which would then inform >> `JSONNodeDumper` or `TextNodeDumper` about the various bits of state in the >> node. With the proposed implementation essentially all of the logic for what >> is in each node has to be duplicated. This is not an objection about the >> current patch, just something I am wondering about. > > This was something I had been pondering when we were doing the great AST dump > refactoring earlier this year, but I didn't think we could get it to be > sufficiently general to suit various purposes. On the one hand, there's > definitely a ton of overlap between any two "dump the AST in this format" > approaches, but on the other hand, different approaches have different > requirements that may change what data is displayed. For instance, the > textual dump will likely write out more information than the JSON dump > because the textual dump is purely for human readability (so having extra > information near the node may be critical) whereas the JSON dump is for > machine readability (so having extra information may or may not be > prohibitive, but hooking up disparate data within the file may be trivial). I see, thanks for the explanation. Someone who is actually going to use the JSON output should probably comment on this patch, but as far as I am concerned I have no objection or additional comment. This is a self-contained feature which is not going to impact anyone else. 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://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
