[PATCH] D90221: Include more attribute details when dumping AST in JSON

Sun Nov 8 00:41:40 PST 2020

aronsky added a comment.

In D90221#2379793 <https://reviews.llvm.org/D90221#2379793>, @aaron.ballman wrote:

> I think the issue is that ASTNodeTraverser is visiting all of the attribute arguments (around ASTNodeTraverse.h:726) and your patch also visits them (as part of what's emitted from ClangAttrEmitter.cpp).

Thanks! I will take a look at that code.

> Unfortunately, I don't think we can accept the patch as-is -- it's incomplete (misses some kinds of attribute arguments), has correctness issues (duplicates some kinds of attribute arguments), and would make it harder to maintain the code moving forward (with the layering issue). I can definitely understand not having a lot of time to investigate the right way to do this, so I spent a bit of time this afternoon working on a patch that gets partway to what I think needs to happen. I put up a pastebin for it here: https://pastebin.com/6ybrPVu6. It does not solve the issue of duplicate traversal, however, and I'm not certain I have more time to put into the patch right now. Perhaps one of us will have the time to take this the last mile -- I think the functionality is needed for JSON dumping.

Thank you! I appreciate your feedback and your work on the issue. I'll take a better look at your patch (from a quick look, it definitely looks more organized and correct than mine), and see if I can understand - and fix - the duplicate traversal issue.

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

https://reviews.llvm.org/D90221