[clang-tidy] Implement clang-tidy check aliases

[leanil]

Summary

Clang warnings can be mapped to virtual check names as aliases. The mappings can be set in the appropriate ClangTidyModules. If the virtual check is enabled, then

• the corresponding warning options get passed to clang, and
• the warning names are replaced with the check alias in the produced messages.

This is an implementation of the proposal here.

[aaron.ballman]

Thank you for working on this -- it's very nice functionality!

clang-tidy/cert/CERTTidyModule.cpp

78

You should declare this as override.

79–81

Can you use try_emplace() instead of insert() -- this removes the need for the braced initializers to create the pair.

82

This check also needs a test case.

[alexfh]

András, that's definitely an interesting idea. However, it might be interesting to explore a more principled approach:

1. Make clang-diagnostic-* checks first-class citizens and take full control of all diagnostics, i.e. disable all Clang diagnostics by default, and enable the ones that correspond to the enabled clang-diagnostic checks.
2. Make aliases first-class citizens (there was a proposal as well, but we didn't arrive to a consensus at that time). That would include the ability to configure an alias name for any check including clang-diagnostic- and clang-analyzer- checks.
3. Use aliases to map clang-diagnostic- checks to check names under cert-, hicpp-, etc.

I didn't carefully consider all possible implications and there may be issues with any or all of the three parts of this, but I think it's worth exploring.

[lebedev.ri]

On a somewhat related note, since this is already talking about aliases

I feel like the current handling of the clang-tidy check aliases needs adjusting.
If one enables all the checks (Checks: '*'), and then disables some of them
on check-by-check basis, if the disabled check has aliases
(cppcoreguidelines-pro-type-vararg vs hicpp-vararg, hicpp-no-array-decay
vs cppcoreguidelines-pro-bounds-array-to-pointer-decay and so on)
each of the aliases must be explicitly be disabled too.

This is rather inconvenient.

If that is intentional, perhaps there could be a config option to either disable
creation/registration of the check aliases altogether, or an option to threat
aliases as a hard link, so anything happening to any of the aliases/base check
would happen to all of them.

(Also, if the check has parameters, and one specifies different parameters for the base check, and the aliases, what happens?)

[xazax.hun]

@leanil : Could you add a test case where the warnings are explicitly disabled in the compilation command and also one where only the aliased warning is explicitly disabled?

In D38171#878814, @lebedev.ri wrote:

I feel like the current handling of the clang-tidy check aliases needs adjusting.
If one enables all the checks (Checks: '*'), and then disables some of them
on check-by-check basis, if the disabled check has aliases
(cppcoreguidelines-pro-type-vararg vs hicpp-vararg, hicpp-no-array-decay
vs cppcoreguidelines-pro-bounds-array-to-pointer-decay and so on)
each of the aliases must be explicitly be disabled too.

That is somewhat intentional I think. Lets imagined you are interested in cppcoreguidelines but not interested in high integrity cpp. It would be great to be able to use Checks: 'cppcoreguidelines*,-hicpp*' without having to worry about the aliases.

In D38171#878814, @lebedev.ri wrote:

If that is intentional, perhaps there could be a config option to either disable
creation/registration of the check aliases altogether, or an option to threat
aliases as a hard link, so anything happening to any of the aliases/base check
would happen to all of them.

I think the source of the problems is that conceptually there are two distinct reasons to disable a check. One reason when the user is not interested in the results regardless what is the category or name of the check. In this case, all aliases should be disabled. The second is when a category of checks (e.g.: a guideline) is not applicable to the code. In this case aliases should not be disabled. So I think a global option would not solve this issue. A better solution might be a per glob notation. So - could mean do not disable aliases and -- could mean disable aliases as well but that is just an example.

All in all, I think this is not related strictly to this issue and I think we should discuss this separately on the mailing list.

In D38171#878814, @lebedev.ri wrote:

(Also, if the check has parameters, and one specifies different parameters for the base check, and the aliases, what happens?)

I think this is a very good point, this needs to be documented somewhere.

[xazax.hun]

In D38171#878808, @alexfh wrote:

András, that's definitely an interesting idea. However, it might be interesting to explore a more principled approach:

1. Make clang-diagnostic-* checks first-class citizens and take full control of all diagnostics, i.e. disable all Clang diagnostics by default, and enable the ones that correspond to the enabled clang-diagnostic checks.

I agree that this is a good idea. I think it could be done in this patch. But to be sure, could you elaborate on what do you mean by first-class citizen?

2. Make aliases first-class citizens (there was a proposal as well, but we didn't arrive to a consensus at that time). That would include the ability to configure an alias name for any check including clang-diagnostic- and clang-analyzer- checks.

Do you have a link to the proposal? What do you mean by the ability to configure? As in having a config file or registering aliases in modules like now? If you mean the former, I think that is better addressed in a follow-up patch.

3. Use aliases to map clang-diagnostic- checks to check names under cert-, hicpp-, etc.

I didn't carefully consider all possible implications and there may be issues with any or all of the three parts of this, but I think it's worth exploring.

[lebedev.ri]

In D38171#880016, @xazax.hun wrote:

In D38171#878814, @lebedev.ri wrote:

I feel like the current handling of the clang-tidy check aliases needs adjusting.
If one enables all the checks (Checks: '*'), and then disables some of them
on check-by-check basis, if the disabled check has aliases
(cppcoreguidelines-pro-type-vararg vs hicpp-vararg, hicpp-no-array-decay
vs cppcoreguidelines-pro-bounds-array-to-pointer-decay and so on)
each of the aliases must be explicitly be disabled too.

That is somewhat intentional I think. Lets imagined you are interested in cppcoreguidelines but not interested in high integrity cpp. It would be great to be able to use Checks: 'cppcoreguidelines*,-hicpp*' without having to worry about the aliases.

In D38171#878814, @lebedev.ri wrote:

If that is intentional, perhaps there could be a config option to either disable
creation/registration of the check aliases altogether, or an option to threat
aliases as a hard link, so anything happening to any of the aliases/base check
would happen to all of them.

I think the source of the problems is that conceptually there are two distinct reasons to disable a check. One reason when the user is not interested in the results regardless what is the category or name of the check. In this case, all aliases should be disabled. The second is when a category of checks (e.g.: a guideline) is not applicable to the code. In this case aliases should not be disabled. So I think a global option would not solve this issue. A better solution might be a per glob notation. So - could mean do not disable aliases and -- could mean disable aliases as well but that is just an example.

All in all, I think this is not related strictly to this issue and I think we should discuss this separately on the mailing list.

In D38171#878814, @lebedev.ri wrote:

(Also, if the check has parameters, and one specifies different parameters for the base check, and the aliases, what happens?)

I think this is a very good point, this needs to be documented somewhere.

A mail [0] posted by @JonasToth is about the similar problem, i think we can continue there.

I *think* he has a point, a some better way to setup aliases, without actually
creating aliased checks is the solution IMO. This way, there won't be any
need to disable all the aliases when completely disabling the checks, and
there won't be multiple config options for the each alias. The legacy support
is the question though.

[0] (https://lists.llvm.org/pipermail/cfe-dev/2017-September/055589.html)

[alexfh]

In D38171#878814, @lebedev.ri wrote:

On a somewhat related note, since this is already talking about aliases

I feel like the current handling of the clang-tidy check aliases needs adjusting.
If one enables all the checks (Checks: '*'), and then disables some of them
on check-by-check basis,

Clang-tidy allows doing so, but I doubt this is a use case we want to encourage. At least, not for a day-to-day use. Most checks require a certain amount of work on the target code and sometimes fine tuning of the check options to be useful for any real codebase. Apart from that, there are checks that give duplicate warnings (some are aliases and some just cover intersecting set of issues), and there are checks that can give conflicting advice. These facts don't combine well with the "enable almost all checks" use case and limit its area of applicability to stress-testing clang-tidy and maybe to initial evaluation of clang-tidy checks on a certain codebase (though going from the other direction and determining the set of style rules for the codebase and then finding which of those are implemented in clang-tidy is a more fruitful approach).

if the disabled check has aliases
(cppcoreguidelines-pro-type-vararg vs hicpp-vararg, hicpp-no-array-decay
vs cppcoreguidelines-pro-bounds-array-to-pointer-decay and so on)
each of the aliases must be explicitly be disabled too.

It seems to be the least confusing of possible options. Gabor made a good point why this is useful as it is.

This is rather inconvenient.

If that is intentional, perhaps there could be a config option to either disable
creation/registration of the check aliases altogether, or an option to threat
aliases as a hard link, so anything happening to any of the aliases/base check
would happen to all of them.

(Also, if the check has parameters, and one specifies different parameters for the base check, and the aliases, what happens?)

[xazax.hun]

In D38171#880022, @lebedev.ri wrote:

A mail [0] posted by @JonasToth is about the similar problem, i think we can continue there.

Great! Could you summarize your points there as well? Thanks in advance.

[lebedev.ri]

In D38171#880025, @xazax.hun wrote:

In D38171#880022, @lebedev.ri wrote:

A mail [0] posted by @JonasToth is about the similar problem, i think we can continue there.

Great! Could you summarize your points there as well? Thanks in advance.

Done.

In D38171#880024, @alexfh wrote:

In D38171#878814, @lebedev.ri wrote:

On a somewhat related note, since this is already talking about aliases

I feel like the current handling of the clang-tidy check aliases needs adjusting.
If one enables all the checks (Checks: '*'), and then disables some of them
on check-by-check basis,

Clang-tidy allows doing so, but I doubt this is a use case we want to encourage.
At least, not for a day-to-day use.

It's much like clang's -Weverything, which is rather convenient, and is
inconveniently 'missing' in gcc, because the devs believe that there would
be complains that it results in new warnings each release,
which is kinda the point of that flag...

Most checks require a certain amount of work on the target code and sometimes
fine tuning of the check options to be useful for any real codebase. Apart from that,
there are checks that give duplicate warnings (some are aliases and some just cover
intersecting set of issues), and there are checks that can give conflicting advice.
These facts don't combine well with the "enable almost all checks" use case and limit
its area of applicability to stress-testing clang-tidy and maybe to initial evaluation of
clang-tidy checks on a certain codebase (though going from the other direction and
determining the set of style rules for the codebase and then finding which of those
are implemented in clang-tidy is a more fruitful approach).

Except the disagreeing checks, same could be said about -Weverything, clang-analyzer,
etc :) Yes, of course it does not just work, and of course needs some further tuning,
and of course the codebase that is being checked needs to be fixed not to warn.

I.e. the use-case is to always have all the warnings enabled, and each time a new check
is triggered, analyze whether it should be kept enabled, or disable it.

[leanil]

Add more tests and minor fixes.

[leanil]

In D38171#880021, @xazax.hun wrote:

In D38171#878808, @alexfh wrote:

András, that's definitely an interesting idea. However, it might be interesting to explore a more principled approach:

1. Make clang-diagnostic-* checks first-class citizens and take full control of all diagnostics, i.e. disable all Clang diagnostics by default, and enable the ones that correspond to the enabled clang-diagnostic checks.

I agree that this is a good idea. I think it could be done in this patch. But to be sure, could you elaborate on what do you mean by first-class citizen?

2. Make aliases first-class citizens (there was a proposal as well, but we didn't arrive to a consensus at that time). That would include the ability to configure an alias name for any check including clang-diagnostic- and clang-analyzer- checks.

Do you have a link to the proposal? What do you mean by the ability to configure? As in having a config file or registering aliases in modules like now? If you mean the former, I think that is better addressed in a follow-up patch.

I also think this sounds good, though I'm not quite sure it can be done in this patch. Anyway, we should definitely specify what we expect from first-class citizen checks. Please correct & extend the list:

• can be enabled or disabled through a unique name
• can have config options
• can have aliases
• inherits ClangTidyCheck

I see the last one as a requirement for handling everything uniformly (without workarounds like this patch), but also as a main point where an alias differs from a "real" check.

[xazax.hun]

Let's also summarize what do we have now and what do we want.

I also think this sounds good, though I'm not quite sure it can be done in this patch. Anyway, we should definitely specify what we expect from first-class citizen checks. Please correct & extend the list:

• can be enabled or disabled through a unique name

This is addressed by the current pathc.

• can have config options

Do you know warnings that have config options? Does this make sense for this featre?

• can have aliases

Does that make sense to be able to add aliases to warning aliases? I

• inherits ClangTidyCheck

I think this is just technical and this does not really affect the users.

In D38171#878808, @alexfh wrote:

1. Make clang-diagnostic-* checks first-class citizens and take full control of all diagnostics, i.e. disable all Clang diagnostics by default, and enable the ones that correspond to the enabled clang-diagnostic checks.

I think this might be a good idea in general. @leanil could you address this point in this patch?

2. Make aliases first-class citizens (there was a proposal as well, but we didn't arrive to a consensus at that time). That would include the ability to configure an alias name for any check including clang-diagnostic- and clang-analyzer- checks.

I am not sure that this makes sense, see my points above.

3. Use aliases to map clang-diagnostic- checks to check names under cert-, hicpp-, etc.

I agree.

[xazax.hun]

One problem to think about when we add all clang-diagnostic as "first or second" class citizen, checkes=* might now enable all the warnings which make no sense and might be surprising to the users. What do you think?

[leanil]

Make clang-diagnostic-* checks first-class citizens and take full control of all diagnostics, i.e. disable all Clang diagnostics by default, and enable the ones that correspond to the enabled clang-diagnostic checks.

(As @alexfh suggested.)

Collecting all the diagnostics seems to be possible from multiple sources, I've decided to make DiagnosticIDs::getAllDiagnostics static.
I also had to replace explicit -W arguments with the new checks in some of the tests.

[leanil]

In D38171#901427, @xazax.hun wrote:

One problem to think about when we add all clang-diagnostic as "first or second" class citizen, checkes=* might now enable all the warnings which make no sense and might be surprising to the users. What do you think?

This is a good point. Should I insert ",-clang-diagnostic*" after any (positive) * ?

[xazax.hun]

In D38171#909346, @leanil wrote:

In D38171#901427, @xazax.hun wrote:

One problem to think about when we add all clang-diagnostic as "first or second" class citizen, checkes=* might now enable all the warnings which make no sense and might be surprising to the users. What do you think?

This is a good point. Should I insert ",-clang-diagnostic*" after any (positive) * ?

@alexfh do you have some thoughts on this?

[alexfh]

In D38171#925929, @xazax.hun wrote:

In D38171#909346, @leanil wrote:

In D38171#901427, @xazax.hun wrote:

One problem to think about when we add all clang-diagnostic as "first or second" class citizen, checkes=* might now enable all the warnings which make no sense and might be surprising to the users. What do you think?

This is a good point. Should I insert ",-clang-diagnostic*" after any (positive) * ?

@alexfh do you have some thoughts on this?

I don't think this particular point is a large concern. As I mentioned multiple times, enabling all checks is almost never useful due to many checks overlapping or producing conflicting advice. The only place I can think of, where -checks=* is useful is in combination with -list-checks, where the presence of clang-diagnostic- entries would be desired anyway.

[alexfh]

And, btw, sorry for the long delay. I've been on travelling / on vacation for the last few weeks.

[malcolm.parsons]

In D38171#927045, @alexfh wrote:

The only place I can think of, where -checks=* is useful is in combination with -list-checks, where the presence of clang-diagnostic- entries would be desired anyway.

Can we replace -list-checks with -list-enabled-checks and -list-all-checks? The current behaviour is confusing and recently caused Chandler to abandon a clang-tidy demo.

[xazax.hun]

In D38171#927046, @alexfh wrote:

And, btw, sorry for the long delay. I've been on travelling / on vacation for the last few weeks.

No problem. Will you have some time to think about the overall concept? Implementation and test wise it looks good to me.
I think this patch is a step in a good direction but of course, it is important for the community to agree on the approach.

test/clang-tidy/warning-check-aliases.cpp

11

There are some redundant empty lines.

[xazax.hun]

clang-tidy/ClangTidyDiagnosticConsumer.cpp

263

I am wondering if this is still a good candidate to be a small vector with this size. Maybe a regular vector with a reserve might be better?

[leanil]

Remove redundant empty lines.
Make list-clang-diagnostics test less strict.
Update getAllDiagnostics to use std::vector.

[leanil]

Does anyone have any more thoughts about this?

[xazax.hun]

@alexfh did you have any chance to think about this change?

[lebedev.ri]

While i don't have a leg to stand on here, i'd be *much* more comfortable if this would be a proper RFC mail in cfe-dev,
that would explore all the possible options (this, and the one from cfe-dev thread clang-tidy and CppCoreGuidelines),
and *explain* why one is chosen, and not the other one.

I still feel like that proposal is more sound.

[alexfh]

I apologize for completely ignoring this for a long time.

I'm totally fine taking full control of clang-diagnostic- "checks", i.e. automatically adding corresponding -W flags and removing all other -W flags (or prepending -Wno-everything).

However, could we split the aliases stuff to a separate patch? My concerns w.r.t. the current implementation:

1. it doesn't allow more than one alias to each diagnostic
2. it's not clear that clang diagnostic aliases need to be separate from aliases for native clang-tidy checks. It might be better to add a more high-level API to register aliases (e.g. registerCheckAlias(<alias name>, <check name>, <custom check options>)).

clang-tidy/ClangTidy.cpp

406

The "clang-diagnostic-" string is now repeated multiple times. Could you pull it out to a constant (or maybe llvm::StringRef getClangDiagnosticPrefix())?

clang-tidy/ClangTidyModule.h

96

Consider using llvm::StringMap.

[alexfh]

In D38171#1036893, @alexfh wrote:

I'm totally fine taking full control of clang-diagnostic- "checks", i.e. automatically adding corresponding -W flags and removing all other -W flags (or prepending -Wno-everything).

However, could we split the aliases stuff to a separate patch?

The other way round. It will be better to split the clang-diagnostic part to a separate patch, since this patch focuses on the aliases.

[alexfh]

test/clang-tidy/misc-suspicious-semicolon-fail.cpp

5

This reminds me that we should also filter out all -W(no-)error.* compiler arguments, if we're to take full control of clang diagnostics and make them behave closer to native checks.

test/clang-tidy/validate-check-names.cpp

2

Which clang diagnostic names fail this test?

clang-tidy/ClangTidy.cpp

 ClangTidyASTConsumerFactory::ClangTidyASTConsumerFactory(
     ClangTidyContext &Context)
     : Context(Context), CheckFactories(new ClangTidyCheckFactories) {
   for (ClangTidyModuleRegistry::iterator I = ClangTidyModuleRegistry::begin(),
                                          E = ClangTidyModuleRegistry::end();
        I != E; ++I) {
     std::unique_ptr<ClangTidyModule> Module(I->instantiate());
     Module->addCheckFactories(*CheckFactories);
+    Module->addWarningCheckAliases(Context.WarningCheckAliases);
   }
 }
 
 static void setStaticAnalyzerCheckerOpts(const ClangTidyOptions &Opts,
                                          AnalyzerOptionsRef AnalyzerOptions) {
   StringRef AnalyzerPrefix(AnalyzerCheckNamePrefix);
   for (const auto &Opt : Opts.CheckOptions) {
     StringRef OptName(Opt.first);

 
 std::vector<std::string> ClangTidyASTConsumerFactory::getCheckNames() {
   std::vector<std::string> CheckNames;
   for (const auto &CheckFactory : *CheckFactories) {
     if (Context.isCheckEnabled(CheckFactory.first))
       CheckNames.push_back(CheckFactory.first);
   }
 
+  for (const auto &Alias : Context.WarningCheckAliases)
+    if (Context.isCheckEnabled(Alias.second))
+      CheckNames.push_back(Alias.second);
+
+  for (const auto &Diag : Context.getEnabledClangDiagnostics())
+    CheckNames.push_back("clang-diagnostic-" + Diag);
+
   for (const auto &AnalyzerCheck : getCheckersControlList(Context))
     CheckNames.push_back(AnalyzerCheckNamePrefix + AnalyzerCheck.first);
 
   std::sort(CheckNames.begin(), CheckNames.end());
   return CheckNames;
 }
 
 ClangTidyOptions::OptionMap ClangTidyASTConsumerFactory::getCheckOptions() {
ArgumentsAdjuster PerFileExtraArgumentsInserter =
           if (I != AdjustedArgs.end() && !StringRef(*I).startswith("-"))
             ++I; // Skip compiler binary name, if it is there.
           AdjustedArgs.insert(I, Opts.ExtraArgsBefore->begin(),
                               Opts.ExtraArgsBefore->end());
         }
         if (Opts.ExtraArgs)
           AdjustedArgs.insert(AdjustedArgs.end(), Opts.ExtraArgs->begin(),
                               Opts.ExtraArgs->end());
+
+        AdjustedArgs.push_back("-Wno-everything");
+        for (const auto &Diag : Context.getEnabledClangDiagnostics())
+          AdjustedArgs.push_back("-W" + Diag);
+
+        for (const auto &Alias : Context.WarningCheckAliases)
+          if (Context.isCheckEnabled(Alias.second))
+            AdjustedArgs.push_back("-W" + Alias.first.str());
+
         return AdjustedArgs;
       };
 
   // Remove plugins arguments.
   ArgumentsAdjuster PluginArgumentsRemover =
       [](const CommandLineArguments &Args, StringRef Filename) {
         CommandLineArguments AdjustedArgs;
         for (size_t I = 0, E = Args.size(); I < E; ++I) {

clang-tidy/ClangTidyDiagnosticConsumer.h

 
 #include "ClangTidyOptions.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Tooling/Core/Diagnostic.h"
 #include "clang/Tooling/Refactoring.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/Timer.h"
 
 namespace clang {
 
 class ASTContext;
 class CompilerInstance;
 namespace ast_matchers {
void setCurrentBuildDirectory(StringRef BuildDirectory) {
     CurrentBuildDirectory = BuildDirectory;
   }
 
   /// \brief Returns build directory of the current translation unit.
   const std::string &getCurrentBuildDirectory() {
     return CurrentBuildDirectory;
   }
 
+  std::vector<std::string> getEnabledClangDiagnostics();
+
+  llvm::DenseMap<llvm::StringRef, llvm::StringRef> WarningCheckAliases;
+
 private:
   // Calls setDiagnosticsEngine() and storeError().
   friend class ClangTidyDiagnosticConsumer;
   friend class ClangTidyPluginAction;
 
   /// \brief Sets the \c DiagnosticsEngine so that Diagnostics can be generated
   /// correctly.
   void setDiagnosticsEngine(DiagnosticsEngine *Engine);

clang-tidy/ClangTidyDiagnosticConsumer.cpp

 
 #include "ClangTidyDiagnosticConsumer.h"
 #include "ClangTidyOptions.h"
 #include "clang/AST/ASTDiagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Frontend/DiagnosticRenderer.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
+#include <algorithm>
 #include <tuple>
 #include <vector>
 using namespace clang;
 using namespace tidy;
 
 namespace {
 class ClangTidyDiagnosticRenderer : public DiagnosticRenderer {
 public:

 StringRef ClangTidyContext::getCheckName(unsigned DiagnosticID) const {
   llvm::DenseMap<unsigned, std::string>::const_iterator I =
       CheckNamesByDiagnosticID.find(DiagnosticID);
   if (I != CheckNamesByDiagnosticID.end())
     return I->second;
   return "";
 }
 
+std::vector<std::string> ClangTidyContext::getEnabledClangDiagnostics() {
+  std::vector<diag::kind> Diags;
+  DiagnosticIDs::getAllDiagnostics(diag::Flavor::WarningOrError, Diags);
+  std::vector<std::string> EnabledClangDiagnostics;
+  for (unsigned DiagID : Diags) {
+    auto Flag = DiagnosticIDs::getWarningOptionForDiag(DiagID).str();
+    if (!Flag.empty() && isCheckEnabled("clang-diagnostic-" + Flag))
+      EnabledClangDiagnostics.push_back(Flag);
+  }
+  std::sort(EnabledClangDiagnostics.begin(), EnabledClangDiagnostics.end());
+  EnabledClangDiagnostics.erase(std::unique(EnabledClangDiagnostics.begin(),
+                                            EnabledClangDiagnostics.end()),
+                                EnabledClangDiagnostics.end());
+  return EnabledClangDiagnostics;
+}
+
 ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(
     ClangTidyContext &Ctx, bool RemoveIncompatibleErrors)
     : Context(Ctx), RemoveIncompatibleErrors(RemoveIncompatibleErrors),
       LastErrorRelatesToUserCode(false), LastErrorPassesLineFilter(false),
       LastErrorWasIgnored(false) {
   IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
   Diags = llvm::make_unique<DiagnosticsEngine>(
       IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs), &*DiagOpts, this,
void ClangTidyDiagnosticConsumer::HandleDiagnostic(
   if (DiagLevel == DiagnosticsEngine::Note) {
     assert(!Errors.empty() &&
            "A diagnostic note can only be appended to a message.");
   } else {
     finalizeLastError();
     StringRef WarningOption =
         Context.DiagEngine->getDiagnosticIDs()->getWarningOptionForDiag(
             Info.getID());
-    std::string CheckName = !WarningOption.empty()
-                                ? ("clang-diagnostic-" + WarningOption).str()
-                                : Context.getCheckName(Info.getID()).str();
+    auto Alias = Context.WarningCheckAliases.find(WarningOption);
+    std::string CheckName;
+    if (Alias != Context.WarningCheckAliases.end() &&
+        Context.isCheckEnabled(Alias->second))
+      CheckName = Alias->second;
+    else
+      CheckName = !WarningOption.empty()
 
     if (CheckName.empty()) {
       // This is a compiler diagnostic without a warning option. Assign check
       // name based on its level.
       switch (DiagLevel) {
       case DiagnosticsEngine::Error:
       case DiagnosticsEngine::Fatal:
         CheckName = "clang-diagnostic-error";

clang-tidy/ClangTidyModule.h

 //===--- ClangTidyModule.h - clang-tidy -------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYMODULE_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYMODULE_H
 
 #include "ClangTidy.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringRef.h"
 #include <functional>
 #include <map>
 #include <string>
 #include <utility>
 
 namespace clang {
 namespace tidy {

 class ClangTidyModule {
 public:
   virtual ~ClangTidyModule() {}
 
   /// \brief Implement this function in order to register all \c CheckFactories
   /// belonging to this module.
   virtual void addCheckFactories(ClangTidyCheckFactories &CheckFactories) = 0;
 
+  /// \brief Implement this function in order to register all warning-check
+  /// aliases belonging to this module.
+  virtual void addWarningCheckAliases(
+      llvm::DenseMap<llvm::StringRef, llvm::StringRef> &WarningCheckAliases) {}
+
   /// \brief Gets default options for checks defined in this module.
   virtual ClangTidyOptions getModuleOptions();
 };
 
 } // end namespace tidy
 } // end namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYMODULE_H

clang-tidy/cert/CERTTidyModule.cpp

void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<FloatLoopCounter>("cert-flp30-c");
     // FIO
     CheckFactories.registerCheck<misc::NonCopyableObjectsCheck>("cert-fio38-c");
     // ERR
     CheckFactories.registerCheck<StrToNumCheck>("cert-err34-c");
     // MSC
     CheckFactories.registerCheck<LimitedRandomnessCheck>("cert-msc30-c");
   }
+
+  void addWarningCheckAliases(llvm::DenseMap<llvm::StringRef, llvm::StringRef>
+                                  &WarningCheckAliases) override {
+    WarningCheckAliases.try_emplace("exceptions", "cert-err54-cpp");
+    WarningCheckAliases.try_emplace("invalid-offsetof", "cert-exp59-cpp");
+  }
 };
 
 } // namespace cert
 
 // Register the MiscTidyModule using this statically initialized variable.
 static ClangTidyModuleRegistry::Add<cert::CERTModule>
     X("cert-module",
       "Adds lint checks corresponding to CERT secure coding guidelines.");
 
 // This anchor is used to force the linker to link in the generated object file
 // and thus register the CERTModule.
 volatile int CERTModuleAnchorSource = 0;
 
 } // namespace tidy
 } // namespace clang

clang-tidy/tool/ClangTidyMain.cpp

configuration can be inspected using -dump-config:
     CheckOptions:
       - key:             some-check.SomeOption
         value:           'some value'
     ...
 
 )");
 
 const char DefaultChecks[] = // Enable these checks by default:
-    "clang-diagnostic-*,"    //   * compiler diagnostics
     "clang-analyzer-*";      //   * Static Analyzer checks
 
 static cl::opt<std::string> Checks("checks", cl::desc(R"(
 Comma-separated list of globs with optional '-'
 prefix. Globs are processed in order of
 appearance in the list. Globs without '-'
 prefix add checks with matching names to the
 set, globs with the '-' prefix remove checks

test/clang-tidy/cert-exp59-cpp.cpp

+// RUN: clang-tidy %s -checks='-*,clang-diagnostic-invalid-offsetof' -- 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy %s -checks='-*,cert-exp59-cpp' -- 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' -check-prefix=CHECK2 %s
+// RUN: clang-tidy %s -- 2>&1 | FileCheck -allow-empty -implicit-check-not='{{warning:|error:}}' -check-prefix=CHECK3 %s
+
+#include <cstddef>
+
+struct D {
+    virtual void f() {}
+    int i;
+};
+
+void f() {
+    size_t Off = offsetof(D, i);
+    //CHECK: :[[@LINE-1]]:18: warning: offset of on non-standard-layout type 'D' [clang-diagnostic-invalid-offsetof]
+    //CHECK2: :[[@LINE-2]]:18: warning: offset of on non-standard-layout type 'D' [cert-exp59-cpp]
+}

test/clang-tidy/custom-diagnostics.cpp

-// RUN: clang-tidy -checks='-*,modernize-use-override,clang-diagnostic-shadow,clang-diagnostic-float-conversion' %s -- | count 0
+// RUN: clang-tidy -checks='-*,modernize-use-override' %s -- | count 0
+//
+// Clang-diagnostic checks completely override extra args:
+// RUN: clang-tidy -checks='-*,modernize-use-override,clang-diagnostic-shadow,clang-diagnostic-float-conversion' \
+// RUN:   -config='{ExtraArgs: ["-Wno-shadow","-Wno-float-conversion","-Wunused-variable"], ExtraArgsBefore: ["-Wno-shadow","-Wno-float-conversion","-Wunused-variable"]}' %s -- \
+// RUN:   | FileCheck -implicit-check-not='{{warning:|error:}}' %s
 //
 // Enable warnings using -config:
 // RUN: clang-tidy -checks='-*,modernize-use-override,clang-diagnostic-shadow,clang-diagnostic-float-conversion' \
 // RUN:   -config='{ExtraArgs: ["-Wshadow","-Wno-unused-variable"], ExtraArgsBefore: ["-Wno-shadow","-Wfloat-conversion","-Wunused-variable"]}' %s -- \
 // RUN:   | FileCheck -implicit-check-not='{{warning:|error:}}' %s
 //
 // ... -extra-arg:
 // RUN: clang-tidy -checks='-*,modernize-use-override,clang-diagnostic-shadow,clang-diagnostic-float-conversion' \

test/clang-tidy/diagnostic.cpp

 // RUN: clang-tidy -checks='-*,modernize-use-override' %s.nonexistent.cpp -- | FileCheck -check-prefix=CHECK1 -implicit-check-not='{{warning:|error:}}' %s
-// RUN: clang-tidy -checks='-*,clang-diagnostic-*,google-explicit-constructor' %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK2 -implicit-check-not='{{warning:|error:}}' %s
-// RUN: clang-tidy -checks='-*,google-explicit-constructor,clang-diagnostic-literal-conversion' %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK3 -implicit-check-not='{{warning:|error:}}' %s
-// RUN: clang-tidy -checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' %s -- -DMACRO_FROM_COMMAND_LINE | FileCheck -check-prefix=CHECK4 -implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor,clang-diagnostic-literal-conversion' %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK2 -implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy -checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' %s -- -DMACRO_FROM_COMMAND_LINE | FileCheck -check-prefix=CHECK3 -implicit-check-not='{{warning:|error:}}' %s
 //
 // Now repeat the tests and ensure no other errors appear on stderr:
 // RUN: clang-tidy -checks='-*,modernize-use-override' %s.nonexistent.cpp -- 2>&1 | FileCheck -check-prefix=CHECK1 -implicit-check-not='{{warning:|error:}}' %s
-// RUN: clang-tidy -checks='-*,clang-diagnostic-*,google-explicit-constructor' %s -- -fan-unknown-option 2>&1 | FileCheck -check-prefix=CHECK2 -implicit-check-not='{{warning:|error:}}' %s
-// RUN: clang-tidy -checks='-*,google-explicit-constructor,clang-diagnostic-literal-conversion' %s -- -fan-unknown-option 2>&1 | FileCheck -check-prefix=CHECK3 -implicit-check-not='{{warning:|error:}}' %s
-// RUN: clang-tidy -checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' %s -- -DMACRO_FROM_COMMAND_LINE 2>&1 | FileCheck -check-prefix=CHECK4 -implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor,clang-diagnostic-literal-conversion' %s -- -fan-unknown-option 2>&1 | FileCheck -check-prefix=CHECK2 -implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy -checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' %s -- -DMACRO_FROM_COMMAND_LINE 2>&1 | FileCheck -check-prefix=CHECK3 -implicit-check-not='{{warning:|error:}}' %s
 //
 // Now create a directory with a compilation database file and ensure we don't
 // use it after failing to parse commands from the command line:
 //
 // RUN: mkdir -p %T/diagnostics/
 // RUN: echo '[{"directory": "%/T/diagnostics/","command": "clang++ -fan-option-from-compilation-database -c %/T/diagnostics/input.cpp", "file": "%/T/diagnostics/input.cpp"}]' > %T/diagnostics/compile_commands.json
 // RUN: cat %s > %T/diagnostics/input.cpp
 // RUN: clang-tidy -checks='-*,modernize-use-override' %T/diagnostics/nonexistent.cpp -- 2>&1 | FileCheck -check-prefix=CHECK1 -implicit-check-not='{{warning:|error:}}' %s
-// RUN: clang-tidy -checks='-*,clang-diagnostic-*,google-explicit-constructor' %T/diagnostics/input.cpp -- -fan-unknown-option 2>&1 | FileCheck -check-prefix=CHECK2 -implicit-check-not='{{warning:|error:}}' %s
-// RUN: clang-tidy -checks='-*,google-explicit-constructor,clang-diagnostic-literal-conversion' %T/diagnostics/input.cpp -- -fan-unknown-option 2>&1 | FileCheck -check-prefix=CHECK3 -implicit-check-not='{{warning:|error:}}' %s
-// RUN: clang-tidy -checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' %T/diagnostics/input.cpp -- -DMACRO_FROM_COMMAND_LINE 2>&1 | FileCheck -check-prefix=CHECK4 -implicit-check-not='{{warning:|error:}}' %s
-// RUN: clang-tidy -checks='-*,clang-diagnostic-*,google-explicit-constructor' %T/diagnostics/input.cpp 2>&1 | FileCheck -check-prefix=CHECK5 -implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor,clang-diagnostic-literal-conversion' %T/diagnostics/input.cpp -- -fan-unknown-option 2>&1 | FileCheck -check-prefix=CHECK2 -implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy -checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' %T/diagnostics/input.cpp -- -DMACRO_FROM_COMMAND_LINE 2>&1 | FileCheck -check-prefix=CHECK3 -implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy -checks='-*,clang-diagnostic-literal-conversion,google-explicit-constructor' %T/diagnostics/input.cpp 2>&1 | FileCheck -check-prefix=CHECK4 -implicit-check-not='{{warning:|error:}}' %s
 
 // CHECK1: error: error reading '{{.*}}nonexistent.cpp' [clang-diagnostic-error]
 // CHECK2: error: unknown argument: '-fan-unknown-option' [clang-diagnostic-error]
-// CHECK3: error: unknown argument: '-fan-unknown-option' [clang-diagnostic-error]
-// CHECK5: error: unknown argument: '-fan-option-from-compilation-database' [clang-diagnostic-error]
+// CHECK4: error: unknown argument: '-fan-option-from-compilation-database' [clang-diagnostic-error]
 
-// CHECK2: :[[@LINE+3]]:9: warning: implicit conversion from 'double' to 'int' changes value from 1.5 to 1 [clang-diagnostic-literal-conversion]
-// CHECK3: :[[@LINE+2]]:9: warning: implicit conversion from 'double' to 'int' changes value
-// CHECK5: :[[@LINE+1]]:9: warning: implicit conversion from 'double' to 'int' changes value
+// CHECK2: :[[@LINE+2]]:9: warning: implicit conversion from 'double' to 'int' changes value from 1.5 to 1 [clang-diagnostic-literal-conversion]
+// CHECK4: :[[@LINE+1]]:9: warning: implicit conversion from 'double' to 'int' changes value
 int a = 1.5;
 
-// CHECK2: :[[@LINE+3]]:11: warning: single-argument constructors must be marked explicit
-// CHECK3: :[[@LINE+2]]:11: warning: single-argument constructors must be marked explicit
-// CHECK5: :[[@LINE+1]]:11: warning: single-argument constructors must be marked explicit
+// CHECK2: :[[@LINE+2]]:11: warning: single-argument constructors must be marked explicit
+// CHECK4: :[[@LINE+1]]:11: warning: single-argument constructors must be marked explicit
 class A { A(int) {} };
 
 #define MACRO_FROM_COMMAND_LINE
-// CHECK4: :[[@LINE-1]]:9: warning: 'MACRO_FROM_COMMAND_LINE' macro redefined
+// CHECK3: :[[@LINE-1]]:9: warning: 'MACRO_FROM_COMMAND_LINE' macro redefined

test/clang-tidy/list-clang-diagnostics.cpp

+// RUN: clang-tidy -checks='-*,clang-diagnostic-*' -list-checks 2>&1 | FileCheck -match-full-lines %s
+// Check that some basic diagnostics are listed as well as some that have aliases.
+
+// CHECK: clang-diagnostic-conversion
+// CHECK: clang-diagnostic-division-by-zero
+// CHECK: clang-diagnostic-exceptions
+// CHECK: clang-diagnostic-invalid-offsetof

test/clang-tidy/misc-suspicious-semicolon-fail.cpp

 // RUN: clang-tidy %s -checks="-*,misc-suspicious-semicolon" -- -DERROR 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=CHECK-ERROR \
 // RUN:       -implicit-check-not="{{warning|error}}:"
-// RUN: clang-tidy %s -checks="-*,misc-suspicious-semicolon,clang-diagnostic*" \
-// RUN:    -- -DWERROR -Wno-everything -Werror=unused-variable 2>&1 \
+// RUN: not clang-tidy %s -checks="-*,misc-suspicious-semicolon,clang-diagnostic-unused-variable" \
+// RUN:   -warnings-as-errors=clang-diagnostic-unused-variable -- -DWERROR 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=CHECK-WERROR \
 // RUN:       -implicit-check-not="{{warning|error}}:"
 
 // Note: This test verifies that, the checker does not emit any warning for
 //       files that do not compile.
 
 bool g();
 
 void f() {
   if (g());
   // CHECK-WERROR: :[[@LINE-1]]:11: warning: potentially unintended semicolon [misc-suspicious-semicolon]
 #if ERROR
   int a
   // CHECK-ERROR: :[[@LINE-1]]:8: error: expected ';' at end of declaration [clang-diagnostic-error]
 #elif WERROR
   int a;
-  // CHECK-WERROR: :[[@LINE-1]]:7: error: unused variable 'a' [clang-diagnostic-unused-variable]
+  // CHECK-WERROR: :[[@LINE-1]]:7: error: unused variable 'a' [clang-diagnostic-unused-variable,-warnings-as-errors]
 #else
 #error "One of ERROR or WERROR should be defined.
 #endif
 }

test/clang-tidy/validate-check-names.cpp

 // Check names may only contain alphanumeric characters, '-', '_', and '.'.
-// RUN: clang-tidy -checks=* -list-checks | grep '^    ' | cut -b5- | not grep -v '^[a-zA-Z0-9_.\-]\+$'
+// RUN: clang-tidy -checks=*,-clang-diagnostic* -list-checks | grep '^    ' | cut -b5- | not grep -v '^[a-zA-Z0-9_.\-]\+$'

test/clang-tidy/warning-check-aliases.cpp

+// RUN: clang-tidy %s -checks='-*,clang-diagnostic-exceptions' -- 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy %s -checks='-*,cert-err54-cpp' -- 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' -check-prefix=CHECK2 %s
+// RUN: clang-tidy %s -- 2>&1 | FileCheck -allow-empty -implicit-check-not='{{warning:|error:}}' -check-prefix=CHECK3 %s
+
+class B {};
+class D : public B {};
+
+void f() {
+  try {
+  } catch (B &X) {
+  } catch (D &Y) {
+  }
+}
+
+//CHECK: :11:12: warning: exception of type 'D &' will be caught by earlier handler [clang-diagnostic-exceptions]
+//CHECK: :10:12: note: for type 'B &'
+
+//CHECK2: :11:12: warning: exception of type 'D &' will be caught by earlier handler [cert-err54-cpp]
+//CHECK2: :10:12: note: for type 'B &'

test/clang-tidy/werrors-diagnostics.cpp

-// RUN: clang-tidy %s -checks='-*,llvm-namespace-comment,clang-diagnostic*' \
-// RUN:   -- -Wunused-variable 2>&1 \
+// RUN: clang-tidy %s -checks='-*,llvm-namespace-comment,clang-diagnostic-unused-variable' -- 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-WARN -implicit-check-not='{{warning|error}}:'
-// RUN: not clang-tidy %s -checks='-*,llvm-namespace-comment,clang-diagnostic*' \
-// RUN:   -warnings-as-errors='clang-diagnostic*' -- -Wunused-variable 2>&1 \
+// RUN: not clang-tidy %s -checks='-*,llvm-namespace-comment,clang-diagnostic-unused-variable' \
+// RUN:   -warnings-as-errors='clang-diagnostic*' -- 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-WERR -implicit-check-not='{{warning|error}}:'
-// RUN: not clang-tidy %s -checks='-*,llvm-namespace-comment,clang-diagnostic*' \
-// RUN:   -warnings-as-errors='clang-diagnostic*' -quiet -- -Wunused-variable 2>&1 \
+// RUN: not clang-tidy %s -checks='-*,llvm-namespace-comment,clang-diagnostic-unused-variable' \
+// RUN:   -warnings-as-errors='clang-diagnostic*' -quiet -- 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-WERR-QUIET -implicit-check-not='{{warning|error}}:'
 
 void f() { int i; }
 // CHECK-WARN: warning: unused variable 'i' [clang-diagnostic-unused-variable]
 // CHECK-WERR: error: unused variable 'i' [clang-diagnostic-unused-variable,-warnings-as-errors]
 // CHECK-WERR-QUIET: error: unused variable 'i' [clang-diagnostic-unused-variable,-warnings-as-errors]
 
 // CHECK-WARN-NOT: treated as
 // CHECK-WERR: 1 warning treated as error
 // CHECK-WERR-QUIET-NOT: treated as