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

[Stephen Kelly via Phabricator via cfe-commits]

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: include/clang/AST/DeclBase.h:1139
+  void dump(raw_ostream &Out, bool Deserialize = false,
+            ASTDumpOutputFormat OutputFormat = ADOF_Default) const;
 
----------------
I think we've talked about this before, but I don't think growing interfaces like this is the best way forward. An enum is a less-good replacement for an object (ie making the user of the API responsible for creating the dumper they want to use).

I think that could be made more convenient in the future. What do you think?


================
Comment at: include/clang/AST/JSONNodeDumper.h:53
+  template <typename Fn> void AddChild(Fn DoAddChild) {
+    return AddChild("inner", DoAddChild);
+  }
----------------
You have 

    !Label.empty() ? Label : "inner";

below. This looks needlessly non-DRY?


================
Comment at: include/clang/AST/JSONNodeDumper.h:130
+// JSON and then dump to a file because the AST is an ordered collection of
+// nodes but our JSON objects are modelled as an unordered container. We can
+// use JSON facilities for constructing the terminal fields in a node because
----------------
I'm no expert in JSON, so I'm not certain if this has implications.

Do entries in a JSON object (ie not an Array-type) have order? Are you 'giving' them order here which is not part of how JSON is usually parsed (eg by third party tools)?

Can you create ordered JSON (arrays) where it is needed? I see that `inner` is already an array, so I'm probably missing the part that is 'modelled as an unordered container'. Can you point that out to me?


================
Comment at: include/clang/AST/JSONNodeDumper.h:161
+      OS << ",\n";
+    OS << Indentation << "\"" << Key << "\": {\n";
+    Indentation += "  ";
----------------
Is it possible this 'textual' JSON generation could produce invalid JSON? One of the things a library solution ensures is balanced braces and valid syntax etc. I'm not familiar enough with JSON to know if it has dark corners.


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

https://reviews.llvm.org/D60910