alexshap added a comment. > 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.
i can use matchers (and actually i did that originally) for everything but can try to explain the rationale behind my (current) approach: Matcher is used for the lookup of the definition of the record by name, the list of ctors is extracted from the definition (Defintion->ctors()) and doesn't require any extra AST traverses. However to handle aggregates and unified initialization we need to traverse the AST and find all the call-sites. It's easy to do it with matchers as well however i was keeping in mind the future potential features (like changing the order of constructor arguments) - in that case one needs just to add another Visit* method to ReorderingASTVisitor instead of matching the full AST against the new Matcher (so i would expect it be faster and the code probably will be more concise). Repository: rL LLVM https://reviews.llvm.org/D23279 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
