[clang-tidy] Added virtual isLanguageVersionSupported to ClangTidyCheck

[njames93]

Summary

Motivated by 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

[Eugene.Zelenko]

Language and/or its standard is checked in other places too. Should all similar places be refactored?

[njames93]

In D75289#1896902, @Eugene.Zelenko wrote:

Language and/or its standard is checked in other places too. Should all similar places be refactored?

They should but I feel they should be in follow up patches, the only reason MakeSmartPtrCheck is in here is because by coincidence it used the same name as what I planned and I got a compile warning about it.
I also don't know exactly where all occurrences are.

[Eugene.Zelenko]

In D75289#1896925, @njames93 wrote:

In D75289#1896902, @Eugene.Zelenko wrote:

Language and/or its standard is checked in other places too. Should all similar places be refactored?

They should but I feel they should be in follow up patches, the only reason MakeSmartPtrCheck is in here is because by coincidence it used the same name as what I planned and I got a compile warning about it.
I also don't know exactly where all occurrences are.

Just grep for getLangOpts().

[njames93]

In D75289#1896929, @Eugene.Zelenko wrote:

In D75289#1896925, @njames93 wrote:

In D75289#1896902, @Eugene.Zelenko wrote:

Language and/or its standard is checked in other places too. Should all similar places be refactored?

They should but I feel they should be in follow up patches, the only reason MakeSmartPtrCheck is in here is because by coincidence it used the same name as what I planned and I got a compile warning about it.
I also don't know exactly where all occurrences are.

Just grep for getLangOpts().

Thanks for that tip, I've put the full implementation in a child revision.

[gribozavr2]

clang-tools-extra/clang-tidy/ClangTidyCheck.h

102

Please update the doc comment on registerMatchers and registerPPCallbacks that there is now a precondition that isLanguageVersionSupported must return true.

[gribozavr2]

clang-tools-extra/clang-tidy/ClangTidyCheck.h

125

Not a critical thing, but I feel like it would be better to put isLanguageVersionSupported before register functions. That logical order makes more sense to a reader that is not familiar with the class.

[njames93]

• Updated docs to include precondition

[njames93]

• Moved isLanguageVersionSupported above register functions

clang-tools-extra/clang-tidy/ClangTidy.cpp

ClangTidyASTConsumerFactory::CreateASTConsumer(
   if (Context.getLangOpts().Modules && OverlayFS != nullptr) {
     auto ModuleExpander = std::make_unique<ExpandModularHeadersPPCallbacks>(
         &Compiler, OverlayFS);
     ModuleExpanderPP = ModuleExpander->getPreprocessor();
     PP->addPPCallbacks(std::move(ModuleExpander));
   }
 
   for (auto &Check : Checks) {
+    if (!Check->isLanguageVersionSupported(Context.getLangOpts()))
+      continue;
     Check->registerMatchers(&*Finder);
     Check->registerPPCallbacks(*SM, PP, ModuleExpanderPP);
   }
 
   std::vector<std::unique_ptr<ASTConsumer>> Consumers;
   if (!Checks.empty())
     Consumers.push_back(Finder->newASTConsumer());
 

clang-tools-extra/clang-tidy/ClangTidyCheck.h

 public:
   /// Initializes the check with \p CheckName and \p Context.
   ///
   /// Derived classes must implement the constructor with this signature or
   /// delegate it. If a check needs to read options, it can do this in the
   /// 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 differ in how they handle
   /// modular #includes:
   ///  - PP is the real Preprocessor. It doesn't walk into modular #includes and
   ///    thus doesn't generate PPCallbacks for their contents.
   ///  - ModuleExpanderPP preprocesses the whole translation unit in the
   ///    non-modular mode, which allows it to generate PPCallbacks not only for
   ///    the main file and textual headers, but also for all transitively
   ///    included modular headers when the analysis runs with modules enabled.
   ///    When modules are not enabled ModuleExpanderPP just points to the real
   ///    preprocessor.
   virtual void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
                                    Preprocessor *ModuleExpanderPP) {}
 
   /// Override this to register AST matchers with \p Finder.
   ///
   /// This should be used by clang-tidy checks that analyze code properties that
   /// dependent on AST knowledge.
   ///
   /// You can register as many matchers as necessary with \p Finder. Usually,
   /// "this" will be used as callback, but you can also specify other callback
   /// classes. Thereby, different matchers can trigger different callbacks.
   ///
+  /// This will only be executed if the function isLanguageVersionSupported
+  /// returns true.
+  ///
   /// If you need to merge information between the different matchers, you can
   /// store these as members of the derived class. However, note that all
   /// matches occur in the order of the AST traversal.
   virtual void registerMatchers(ast_matchers::MatchFinder *Finder) {}
 
   /// ``ClangTidyChecks`` that register ASTMatchers should do the actual
   /// work in here.
   virtual void check(const ast_matchers::MatchFinder::MatchResult &Result) {}
 
   /// Add a diagnostic with the check's name.
   DiagnosticBuilder diag(SourceLocation Loc, StringRef Description,
                          DiagnosticIDs::Level Level = DiagnosticIDs::Warning);
 
   /// Should store all options supported by this check with their
   /// current values or default values for options that haven't been overridden.
   ///
   /// The check should use ``Options.store()`` to store each option it supports
   /// whether it has the default value or it has been overridden.
   virtual void storeOptions(ClangTidyOptions::OptionMap &Options) {}
 
   /// Provides access to the ``ClangTidyCheck`` options via check-local
   /// names.
   ///
   /// Methods of this class prepend ``CheckName + "."`` to translate check-local
   /// option names to global option names.
   class OptionsView {
   public:
     /// Initializes the instance using \p CheckName + "." as a prefix.
     OptionsView(StringRef CheckName,
                 const ClangTidyOptions::OptionMap &CheckOptions);
 
     /// Read a named option from the ``Context``.
     ///
     /// Reads the option with the check-local name \p LocalName from the
     /// ``CheckOptions``. If the corresponding key is not present, returns

clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h

protected:
 
   /// Returns matcher that match with different smart pointer types.
   ///
   /// Requires to bind pointer type (qualType) with PointerType string declared
   /// in this class.
   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[];
 
 private:
   std::unique_ptr<utils::IncludeInserter> Inserter;
   const utils::IncludeSorter::IncludeStyle IncludeStyle;
   const std::string MakeSmartPtrFunctionHeader;
   const std::string MakeSmartPtrFunctionName;

clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp

 bool MakeSmartPtrCheck::isLanguageVersionSupported(
     const LangOptions &LangOpts) const {
   return LangOpts.CPlusPlus11;
 }
 
 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(
       cxxConstructExpr(hasDeclaration(decl(unless(isPublic())))))));
 
   auto IsPlacement = hasAnyPlacementArg(anything());
 
   Finder->addMatcher(

clang-tools-extra/clangd/ParsedAST.cpp

ASTDiags.setLevelAdjuster([&CTContext](DiagnosticsEngine::Level DiagLevel,
             return DiagnosticsEngine::Ignored;
           }
         }
       }
       return DiagLevel;
     });
     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);
       Check->registerMatchers(&CTFinder);
     }
   }
 
   // Add IncludeFixer which can recover diagnostics caused by missing includes

clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h

std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &Compiler,
 
     Preprocessor *PP = &Compiler.getPreprocessor();
 
     // Checks must be created here, _after_ `Context` has been initialized, so
     // that check constructors can access the context (for example, through
     // `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);
     }
     return Finder.newASTConsumer();
   }
 
   SmallVectorImpl<std::unique_ptr<ClangTidyCheck>> &Checks;
   ast_matchers::MatchFinder &Finder;