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

Sun May 12 13:47:02 PDT 2019

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 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?
> I think that may be a good future improvement, yes.
> 
> One thing to take on balance when considering this for the future is that `dump()` can be called from within a debugger and that's a bit harder to accomplish with an object interface. I'm not certain that will be a big issue for my use case, but it may matter to some folks.
I don't think "it may matter to some folks." is a good guideline to use when designing an API. I have a patch which moves ASTDumper to its own header which I have just uploaded here: https://reviews.llvm.org/D61835

If additional args to `dump()` are mainly for debugging, then adding new args which are not for debugging doesn't seem right. 

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

https://reviews.llvm.org/D60910