gribozavr2 marked 2 inline comments as done. gribozavr2 added inline comments.
================ Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:335 + template <bool has_same_type> struct is_same_method_impl { + static bool isSameMethod(...) { return false; } + }; ---------------- ymandel wrote: > Why use var-args rather than spelling out the type arguments like you have on > lines 339-341 or, simpler, line 351? Because all of those things are not relevant. However I do see your point, that in some sense it is a trick to reduce the number of characters, but somewhat hurting readability, so I changed the signature to be explicit. ================ Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:339 + template <> struct is_same_method_impl<true> { + template <typename FirstTy, typename FirstResult, typename... FirstParams, + typename SecondTy, typename SecondResult, ---------------- ymandel wrote: > Given that we've established them to be the same type, why two sets of > template arguments? Indeed, simplified the code now! ================ Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:637 case Stmt::CLASS##Class: \ - TRY_TO(WalkUpFrom##CLASS(static_cast<CLASS *>(S))); break; + if (isSameMethod(&RecursiveASTVisitor::Traverse##CLASS, \ + &Derived::Traverse##CLASS)) { \ ---------------- ymandel wrote: > Do you explain this logic somewhere? Or do you feel it will be obvious to the > reader? (I don't have a good intuition about this class to judge). Added a comment. The RecursiveASTVisitor is full of tricky logic so no, none of this is obvious. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82486/new/ https://reviews.llvm.org/D82486 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits