[PATCH] D28729: [clang-tidy] Add -enable-alpha-checks command

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D28729#646876, @Prazek wrote:

> In https://reviews.llvm.org/D28729#646758, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D28729#646560, @alexfh wrote:
> >
> > > In https://reviews.llvm.org/D28729#646555, @aaron.ballman wrote:
> > >
> > > > In https://reviews.llvm.org/D28729#646548, @alexfh wrote:
> > > >
> > > > > As discussed with the Static Analyzer maintainers, alpha checkers are completely unsupported and are suitable for very early testing only. We had problems with them routinely, that's why I disabled alpha checkers in clang-tidy completely. I don't think there should be a user-visible way to enable them. Developers can locally change the code to get access to alpha checkers, but released binaries shouldn't provide this possibility.
> > > >
> > > >
> > > > That's good to know -- should it be documented a bit more explicitly,
> > >
> > >
> > > Yes, a comment to that effect near the relevant code wouldn't hurt. Or do you have other suggestions?
> >
> >
> > A comment in the code would be okay, but I think that removing all public mention of the alpha checkers (help text and website) would also be useful; I would not have thought that a production compiler would carry these checks for this many years if they weren't stable and useful.
>
>
> I don't quite understand. Are you suggesting:
>
> - removing help text
> - adding cl::hidden


I'm suggesting removing help text.

> What website are you talking about?

Things like this: http://clang-analyzer.llvm.org/alpha_checks.html

To me, there's a difference between "prevented from being on by default" and "completely unsupported". If they're completely unsupported, we probably shouldn't expose them more easily, but we may want to clarify this in user-facing documentation and help text.

> 
> 
>>> 
>>> 
>>>>   or perhaps the alpha checks should be removed until they're fit for public consumption? Some of those alpha checks have been in the product for a long time, and if they're so unstable that we cannot expose them in a user-friendly fashion, perhaps they don't belong yet?
>>> 
>>> As discussed with SA folks, alpha checkers are convenient for them to develop new checks, but shouldn't be exposed to users. Some of these experimental checkers might deserve being moved out of alpha or removed completely, but that should be reported to and discussed with the SA maintainers.
>> 
>> Agreed.
> 
> I don't agree that alpha checkers are that bad. When I was testing it year ago there were only few false positives, probably in 2 checkers.

If the maintainers of the static analyzer think they shouldn't be exposed to users, I'm inclined to follow that advice.


https://reviews.llvm.org/D28729