[PATCH] D102213: [ASTMatchers] Add forCallable(), a generalization of forFunction().

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed May 12 11:39:27 PDT 2021


aaron.ballman added a comment.

In D102213#2754495 <https://reviews.llvm.org/D102213#2754495>, @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>` which is a smaller set of matchers. Eg., `forFunction(hasName())` is valid code but `forCallable(hasName())` is not because `BlockDecl` isn't a `NamedDecl`. So, like, I suspect that not a lot of code expects this to be **//the//** function, but a lot of code expects it to be **//a//** function. See how much types did I have to change just to get the infinite loop checker to compile in D102214 <https://reviews.llvm.org/D102214>.
>
> If we're ok with such compatibility breakage then i'm all for it.

That's a fair point. We try to not let changes break existing code, but we also don't make any stability guarantees that prevent us from doing so when that's the right answer. Would this breakage be loud for users of the C++ interface, or would the behavior silently change so that anyone who fails to add a `namedDecl()` in the right place stops getting expected results?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102213/new/

https://reviews.llvm.org/D102213



More information about the cfe-commits mailing list