aaron.ballman added inline comments. ================ Comment at: clang-tidy/misc/MiscTidyModule.cpp:58 @@ -57,3 +56,1 @@ - CheckFactories.registerCheck<InefficientAlgorithmCheck>( - "misc-inefficient-algorithm"); CheckFactories.registerCheck<MacroParenthesesCheck>( ---------------- alexfh wrote: > alexfh wrote: > > aaron.ballman wrote: > > > This will break projects that enable the misc-inefficient-algorithm check > > > (which clang 3.7 exposed). Is there a reason to not keep this check > > > registered under this name? > > > > > > (Perhaps a follow-up patch to allow deprecation of check names so that > > > users are given guidance would make sense.) > > I don't feel strongly, but I'm somewhat reluctant to keep old check names. > > With every new clang-tidy version someone starts using on a project, they > > need to carefully look at the list of checks and select relevant ones > > anyway. I think, adding facilities for deprecating checks and keeping old > > names is not going to help much, but will certainly add support burden for > > us. > But we certainly need to mention the rename in the release notes for 3.8. > I don't feel strongly, but I'm somewhat reluctant to keep old check names. > With every new clang-tidy version someone starts using on a project, they > need to carefully look at the list of checks and select relevant ones anyway. > I think, adding facilities for deprecating checks and keeping old names is > not going to help much, but will certainly add support burden for us.
I'm more worried about upgrading existing uses than initiating new uses on a project. If my build system enabled this check for my project, then upgrading clang-tidy will cause that build to break because of an unknown check name, won't it? I think it's reasonable to do that if there's compelling reason (e.g., we remove a check entirely because it's no longer useful for some reason), but I'd like to avoid gratuitously breaking changes because it adds a barrier to people's upgrade paths. Oye. I just tested this out and the results were...surprisingly unhelpful. ``` e:\llvm\2015>clang-tidy -checks=misc-hahahaha-nope E:\Desktop\test.cpp -- e:\llvm\2015> ``` So it seems we don't currently diagnose providing unknown check names at all, which would make this a silently breaking change (existing uses will no longer trigger the check *and* they won't trigger any diagnostic mentioning that the check isn't known). :-( http://reviews.llvm.org/D16248 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits