[clang-tools-extra] 39c4246 - [clang-tidy] Added virtual isLanguageVersionSupported to ClangTidyCheck

Nathan James via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 28 05:05:30 PST 2020


Author: Nathan James
Date: 2020-02-28T13:05:05Z
New Revision: 39c4246e1e5bb76b1bca2e4b0d0ebc4005d75c32

URL: https://github.com/llvm/llvm-project/commit/39c4246e1e5bb76b1bca2e4b0d0ebc4005d75c32
DIFF: https://github.com/llvm/llvm-project/commit/39c4246e1e5bb76b1bca2e4b0d0ebc4005d75c32.diff

LOG: [clang-tidy] Added virtual isLanguageVersionSupported to ClangTidyCheck

Summary:
Motivated by [[ https://bugs.llvm.org/show_bug.cgi?id=45045 | Tune inspections to a specific C++ standard. ]]
Moves the isLanguageVersionSupported virtual function from `MakeSmartPtrCheck` to the base `ClangTidyCheck` class.
This will disable registering matchers or pp callbacks on unsupported language versions for a check.
Having it as a standalone function is cleaner than manually disabling the check in the register function and should hopefully
encourage check developers to actually restrict the check based on language version.
As an added bonus this could enable automatic detection of what language version a check runs on for the purpose of documentation generation

Reviewers: aaron.ballman, gribozavr2, Eugene.Zelenko, JonasToth, alexfh, hokein

Reviewed By: gribozavr2

Subscribers: xazax.hun, jkorous, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D75289

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/ClangTidy.cpp
    clang-tools-extra/clang-tidy/ClangTidyCheck.h
    clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
    clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h
    clang-tools-extra/clangd/ParsedAST.cpp
    clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp
index 3bdcdd2c4727..52c217b649f5 100644
--- a/clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -409,6 +409,8 @@ ClangTidyASTConsumerFactory::CreateASTConsumer(
   }
 
   for (auto &Check : Checks) {
+    if (!Check->isLanguageVersionSupported(Context.getLangOpts()))
+      continue;
     Check->registerMatchers(&*Finder);
     Check->registerPPCallbacks(*SM, PP, ModuleExpanderPP);
   }

diff  --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.h b/clang-tools-extra/clang-tidy/ClangTidyCheck.h
index 440293f5beaa..f274cf9d48c2 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -53,11 +53,24 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
   /// constructor using the Options.get() methods below.
   ClangTidyCheck(StringRef CheckName, ClangTidyContext *Context);
 
+  /// Override this to disable registering matchers and PP callbacks if an
+  /// invalid language version is being used.
+  ///
+  /// For example if a check is examining overloaded functions then this should
+  /// be overridden to return false when the CPlusPlus flag is not set in
+  /// \p LangOpts.
+  virtual bool isLanguageVersionSupported(const LangOptions &LangOpts) const {
+    return true;
+  }
+
   /// Override this to register ``PPCallbacks`` in the preprocessor.
   ///
   /// This should be used for clang-tidy checks that analyze preprocessor-
   /// dependent properties, e.g. include directives and macro definitions.
   ///
+  /// This will only be executed if the function isLanguageVersionSupported
+  /// returns true.
+  ///
   /// There are two Preprocessors to choose from that 
diff er in how they handle
   /// modular #includes:
   ///  - PP is the real Preprocessor. It doesn't walk into modular #includes and
@@ -80,6 +93,9 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
   /// "this" will be used as callback, but you can also specify other callback
   /// classes. Thereby, 
diff erent matchers can trigger 
diff erent callbacks.
   ///
+  /// This will only be executed if the function isLanguageVersionSupported
+  /// returns true.
+  ///
   /// If you need to merge information between the 
diff erent matchers, you can
   /// store these as members of the derived class. However, note that all
   /// matches occur in the order of the AST traversal.

diff  --git a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
index 20a7c45c7cbc..a41a5a2ca25d 100644
--- a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -68,17 +68,12 @@ bool MakeSmartPtrCheck::isLanguageVersionSupported(
 void MakeSmartPtrCheck::registerPPCallbacks(const SourceManager &SM,
                                             Preprocessor *PP,
                                             Preprocessor *ModuleExpanderPP) {
-  if (isLanguageVersionSupported(getLangOpts())) {
     Inserter = std::make_unique<utils::IncludeInserter>(SM, getLangOpts(),
                                                          IncludeStyle);
     PP->addPPCallbacks(Inserter->CreatePPCallbacks());
-  }
 }
 
 void MakeSmartPtrCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
-  if (!isLanguageVersionSupported(getLangOpts()))
-    return;
-
   // Calling make_smart_ptr from within a member function of a type with a
   // private or protected constructor would be ill-formed.
   auto CanCallCtor = unless(has(ignoringImpCasts(

diff  --git a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h
index 30fdab8e9234..1f738737ab26 100644
--- a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h
@@ -41,7 +41,7 @@ class MakeSmartPtrCheck : public ClangTidyCheck {
   virtual SmartPtrTypeMatcher getSmartPointerTypeMatcher() const = 0;
 
   /// Returns whether the C++ version is compatible with current check.
-  virtual bool isLanguageVersionSupported(const LangOptions &LangOpts) const;
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override;
 
   static const char PointerType[];
 

diff  --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
index c8c06fd98ccf..36a9c47f7a9d 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -301,6 +301,8 @@ ParsedAST::build(std::unique_ptr<clang::CompilerInvocation> CI,
     });
     Preprocessor *PP = &Clang->getPreprocessor();
     for (const auto &Check : CTChecks) {
+      if (!Check->isLanguageVersionSupported(CTContext->getLangOpts()))
+        continue;
       // FIXME: the PP callbacks skip the entire preamble.
       // Checks that want to see #includes in the main file do not see them.
       Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);

diff  --git a/clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h b/clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
index eb138b2bc17d..9599576592af 100644
--- a/clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
+++ b/clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
@@ -67,6 +67,8 @@ class TestClangTidyAction : public ASTFrontendAction {
     // `getLangOpts()`).
     CheckFactory<CheckTypes...>::createChecks(&Context, Checks);
     for (auto &Check : Checks) {
+      if (!Check->isLanguageVersionSupported(Context.getLangOpts()))
+        continue;
       Check->registerMatchers(&Finder);
       Check->registerPPCallbacks(Compiler.getSourceManager(), PP, PP);
     }


        


More information about the cfe-commits mailing list