alexfh added inline comments.
================
Comment at: clang-tidy/misc/MiscTidyModule.cpp:58
@@ -57,3 +56,1 @@
- CheckFactories.registerCheck<InefficientAlgorithmCheck>(
- "misc-inefficient-algorithm");
CheckFactories.registerCheck<MacroParenthesesCheck>(
----------------
aaron.ballman wrote:
> 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). :-(
> 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?
Only in one case: when you have just one check enabled. Clang-tidy's -checks=
option is a **filter**, not a **list**, so it can't detect a presence of
invalid check names there. We could add this detection, probably (e.g. if
removal of a glob from the list doesn't change anything), and issue a warning,
but there is no reason to fail hard, when the check filter contains invalid
entries, IMO.
http://reviews.llvm.org/D16248
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
