aaron.ballman added a subscriber: Tyker. aaron.ballman added a comment. In D90221#2381325 <https://reviews.llvm.org/D90221#2381325>, @aronsky wrote:
> 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. You're welcome! FWIW, it looks like @Tyker just committed some changes in d093401a2617d3c46aaed9eeaecf877e3ae1a9f1 <https://reviews.llvm.org/rGd093401a2617d3c46aaed9eeaecf877e3ae1a9f1> that will probably impact this work. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90221/new/ https://reviews.llvm.org/D90221 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
