vsavchenko added a comment. In D102213#2755963 <https://reviews.llvm.org/D102213#2755963>, @NoQ wrote:
> I totally agree that changing forFunction() to work correctly is the right > solution but backwards compatibility breakage is very real because > forFunction() accepts a Matcher<FunctionDecl> whereas forCallable() accepts > Matcher<Decl> Yeah, I see now, it's not that straightforward. Parameter type difference can be combated even keeping backward compatibility (so to speak). You can do `AST_MATCHER_P_OVERLOAD` (like it's done for `hasPrefix`, for example) to have two overloads of `forFunction`: one for `Matcher<FunctionDecl>` (aka old implementation) and one for `Matcher<Decl>` for new implementation. I didn't check it, but it probably should work. It is not elegant though and I'm not pushing it. > The breakage is loud; the code will no longer compile when the intermediate > decl() (or namedDecl(), or whatever) is not present. The more annoying part > is that when you add namedDecl() back (or if you had it spelled out this way > from the start, which doesn't make much sense but is valid and shorter than > spelling out functionDecl()), your Node.get<FunctionDecl>() in the match > callback will silently start returning null (on anything that isn't a > functionDecl()) which may lead to unexpected crashes (previously there was no > match at all, now there's a match but the node isn't of the expected type). > So a relatively distant piece of code will require manual audit in order to > address the potential breakage. The breakage example has a bit weird precondition IMO. In that scenario, the user had to use `decl` or `namedDecl` where they actually meant `functionDecl` AND they should run their matchers on code with blocks. And even if it does happen to someone, I think it's not going to be very painful because a simple dump of the node will show what's going on. > Additionally, forCallable(functionDecl()) is not equivalent to forFunction() > (the former silently matches less stuff). I'm not sure I understand this one. Can you please show an example where one matches something that the other doesn't? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102213/new/ https://reviews.llvm.org/D102213 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
