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

Reply via email to