[PATCH] D54061: [clang-tidy] run() doesn't update the SourceManager.

Jonas Toth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 6 01:26:37 PST 2018


JonasToth added a comment.

In https://reviews.llvm.org/D54061#1288395, @sammccall wrote:

> In https://reviews.llvm.org/D54061#1286956, @JonasToth wrote:
>
> > > Theoretically, we could replace `ClangTidyCheck::check` with `ClangTidyCheck::run`, but I'm not sure it is worth, `ClangTidyCheck::check` is a public API, and is widely-used (for all clang-tidy checks), replacing it requires large changes (although it is one-line change), it might break downstream clang-tidy checks.
> >
> > We can add a deprecation warning and remove the `check` method in the
> >  next version?
>
>
> As Haojian says, this is a lot of churn.
>  I don't think the benefit of pushing people to migrate, even with a grace period, is clear.
>
> Additionally it's *possible* that future refactorings may mean we want to run additional "framework" logic when the MatchFinder calls us, and so requiring `ClangTidy::check` and `MatchCallback::run` to be the same function seems a little risky.
>
> I'll add a comment explaining why the two functions exist.


Alright.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54061





More information about the cfe-commits mailing list