[PATCH] D23279: clang-reorder-fields

Daniel Jasper via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 22 13:06:19 PDT 2016


djasper added a subscriber: djasper.
djasper added a comment.

Sorry for the long silence. Manuel is currently out on vacation. I am not entirely sure how we want to make progress at this point. There seems to be consensus that in the mid-term, this should go into the clang-refactor tool. However, it might take a bit for that to be there (we have to carefully design that, I think). In the meantime, I don't know whether we should check this in as a standalone-tool and then merge into clang-refactor once ready. Ben, what do you think? Also, I guess we could at the very least start with a proper review of the code. That seems to be useful irrespective of how where the code ends up in the end.

My only high-level comment so far (will try to review in more detail later) is: I find it strange to mix ASTMatchers and a RecursiveASTVisitor in the same tool. It seem that if you are using ASTMatchers internally for other things, you should also use ASTMatchers to find all the constructors etc. That should (I hope) make the code a bit simpler.


Repository:
  rL LLVM

https://reviews.llvm.org/D23279





More information about the cfe-commits mailing list