https://github.com/ilya-biryukov commented:
Thanks a lot for this! I have left quite a few comments, mostly NITs. A few things that really stood out and I think we should fix: - increasing the number of macros and signatures we use seems like a bad trade-off. It's much better to have more parameters and ignore them in most cases, but only have a single macro to deal with as a return. The lower complexity of the code using the macros is really something we should optimize for. - the code is using the ASTMatchers terminology despite switching away from it. I find it to be causing more confusion than good. We are really replacing matchers with AST traversal, so I left a few suggestions that go in the direction of switching from - some code has been moved around, making it a little hard to diff between the original and updated version. I suspect it would not compile otherwise, but I am happy to help find workarounds to keep the code compiling and keep every single function and class that reimplemented the original code at the same place. I think this should really help to ensure the code is equivalent and will make it easier to review. https://github.com/llvm/llvm-project/pull/124554 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
