aaron.ballman added a comment.

In D112491#3096935 <https://reviews.llvm.org/D112491#3096935>, @sammccall wrote:

> This looks really sensible to me too. Some naming bikeshedding, but my main 
> question is migration.
>
> Supporting two things is indeed annoying and confusing. I agree it's not 
> worth keeping the old way forever just to avoid writing `refersToVarDecl`.
> The question is: how soon can we get rid of it? We should consider whether we 
> can do it in this patch: just replace the old matcher with this one.
>
> AFAICT the old matcher has no uses in-tree beyond tests/registration. FWIW it 
> has no uses in our out-of-tree repo either (which generally is a heavy 
> matchers user).
> I'm sure it has users somewhere, but probably few, and the mechanical 
> difficulty of updating them is low. Given that I think we should just break 
> them, rather than deal with overloading and deprecation warnings and future 
> cleanup just to put off breaking them for one more release cycle.
>
> This is a tradeoff, to me it seems but reasonable people can disagree on the 
> importance of stability. Aaron, happy to go with whatever you decide.

I did some looking around to see if I could find any uses of the matcher in the 
wild, and I can't spot any that aren't in a Clang fork. I think there's plenty 
of time for us to get feedback from early adopters if removing the old 
interface causes severe pain for anyone. So I'm on board with replacing the old 
APIs, but we should definitely have a release note explaining what's happening.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112491

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to