[clang-tidy] Refactor common code from the Noexcept*Checks into `NoexceptFunctionCheck`

[AMS21]

Summary

As discussed in the https://reviews.llvm.org/D148697 review.

[PiotrZSL]

Overall good, my main concern is base class name & location.

clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.h

23

Consider putting "Base" into a name, so that it woudn't be considered a full check, and it can be in this module, no need to put it into utils

24–26

we got c++17, so delegate constructor.
using utils::NoexceptFunctionCheck::NoexceptFunctionCheck;

27–28

consider moving this into base class

clang-tools-extra/clang-tidy/utils/NoexceptFunctionCheck.h

34–38 ↗

(On Diff #532388)

you can move those into protected.

[AMS21]

Implement suggested changes

[PiotrZSL]

The is compiler error in CI "Cannot find source file: BaseNoexceptFunctionCheck.cpp".
Consider naming it NoexceptFunctionBase.cpp or NoexceptFunctionBaseCheck.cpp unless there are already other .cpp files in some other modules that start with Base, in such case its fine.
Main reason for this is to somehow show others that this file is not an full check, but more a base for some checks.

clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.h

29

you can put those all as private here in derived class.

[AMS21]

So somehow I didn't add the actual BaseNoexceptFunctionCheck files, thats why the build failed.

Also implemented the two new suggestions.

[AMS21]

If there are no more problems, I would kindly ask for someone to push this no my behalf :)

clang-tools-extra/clang-tidy/performance/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   FrontendOpenMP
   Support
   )
 
 add_clang_library(clangTidyPerformanceModule
   AvoidEndlCheck.cpp
+  BaseNoexceptFunctionCheck.cpp
   FasterStringFindCheck.cpp
   ForRangeCopyCheck.cpp
   ImplicitConversionInLoopCheck.cpp
   InefficientAlgorithmCheck.cpp
   InefficientStringConcatenationCheck.cpp
   InefficientVectorOperationCheck.cpp
   MoveConstArgCheck.cpp
   MoveConstructorInitCheck.cpp

clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.h

 //===--- NoexceptDestructorCheck.h - clang-tidy -----------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTDESTRUCTORCHECK_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTDESTRUCTORCHECK_H
 
 #include "../ClangTidyCheck.h"
-#include "../utils/ExceptionSpecAnalyzer.h"
+#include "BaseNoexceptFunctionCheck.h"
 
 namespace clang::tidy::performance {
 
 /// The check flags destructors not marked with `noexcept` or marked
 /// with `noexcept(expr)` where `expr` evaluates to `false`
 /// (but is not a `false` literal itself).
 ///
 /// For the user-facing documentation see:
 /// https://clang.llvm.org/extra/clang-tidy/checks/performance/noexcept-destructor.html
-class NoexceptDestructorCheck : public ClangTidyCheck {
+class NoexceptDestructorCheck : public BaseNoexceptFunctionCheck {
 public:
-  NoexceptDestructorCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  using BaseNoexceptFunctionCheck::BaseNoexceptFunctionCheck;
+
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
-  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
-    return LangOpts.CPlusPlus11 && LangOpts.CXXExceptions;
-  }
-  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
-  std::optional<TraversalKind> getCheckTraversalKind() const override {
-    return TK_IgnoreUnlessSpelledInSource;
-  }
 
-private:
-  utils::ExceptionSpecAnalyzer SpecAnalyzer;
+protected:
+  DiagnosticBuilder
+  reportMissingNoexcept(const FunctionDecl *FuncDecl) final override;
+  void reportNoexceptEvaluatedToFalse(const FunctionDecl *FuncDecl,
+                                      const Expr *NoexceptExpr) final override;
 };
 
 } // namespace clang::tidy::performance
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTDESTRUCTORCHECK_H

clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.cpp

 //===--- NoexceptDestructorCheck.cpp - clang-tidy -------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "NoexceptDestructorCheck.h"
-#include "../utils/LexerUtils.h"
-#include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang::tidy::performance {
 
 void NoexceptDestructorCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(
-      functionDecl(unless(isDeleted()), cxxDestructorDecl()).bind("decl"),
+  Finder->addMatcher(functionDecl(unless(isDeleted()), cxxDestructorDecl())
+                         .bind(BindFuncDeclName),
                      this);
 }
 
-void NoexceptDestructorCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>("decl");
-  assert(FuncDecl);
-
-  if (SpecAnalyzer.analyze(FuncDecl) !=
-      utils::ExceptionSpecAnalyzer::State::Throwing)
-    return;
-
-  // Don't complain about nothrow(false), but complain on nothrow(expr)
-  // where expr evaluates to false.
-  const auto *ProtoType = FuncDecl->getType()->castAs<FunctionProtoType>();
-  const Expr *NoexceptExpr = ProtoType->getNoexceptExpr();
-  if (NoexceptExpr) {
-    NoexceptExpr = NoexceptExpr->IgnoreImplicit();
-    if (!isa<CXXBoolLiteralExpr>(NoexceptExpr)) {
-      diag(NoexceptExpr->getExprLoc(),
-           "noexcept specifier on the destructor evaluates to 'false'");
-    }
-    return;
-  }
-
-  auto Diag = diag(FuncDecl->getLocation(), "destructors should "
-                                            "be marked noexcept");
+DiagnosticBuilder
+NoexceptDestructorCheck::reportMissingNoexcept(const FunctionDecl *FuncDecl) {
+  return diag(FuncDecl->getLocation(), "destructors should "
+}
 
-  // Add FixIt hints.
-  const SourceManager &SM = *Result.SourceManager;
-
-  const SourceLocation NoexceptLoc =
-      utils::lexer::getLocationForNoexceptSpecifier(FuncDecl, SM);
-  if (NoexceptLoc.isValid())
-    Diag << FixItHint::CreateInsertion(NoexceptLoc, " noexcept ");
+void NoexceptDestructorCheck::reportNoexceptEvaluatedToFalse(
+    const FunctionDecl *FuncDecl, const Expr *NoexceptExpr) {
+  diag(NoexceptExpr->getExprLoc(),
+       "noexcept specifier on the destructor evaluates to 'false'");
 }
 
 } // namespace clang::tidy::performance

clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.h

 //===--- NoexceptMoveConstructorCheck.h - clang-tidy-------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTMOVECONSTRUCTORCHECK_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTMOVECONSTRUCTORCHECK_H
 
 #include "../ClangTidyCheck.h"
-#include "../utils/ExceptionSpecAnalyzer.h"
+#include "BaseNoexceptFunctionCheck.h"
 
 namespace clang::tidy::performance {
 
 /// The check flags user-defined move constructors and assignment operators not
 /// marked with `noexcept` or marked with `noexcept(expr)` where `expr`
 /// evaluates to `false` (but is not a `false` literal itself).
 ///
 /// Move constructors of all the types used with STL containers, for example,
 /// need to be declared `noexcept`. Otherwise STL will choose copy constructors
 /// instead. The same is valid for move assignment operations.
 ///
 /// For the user-facing documentation see:
 /// https://clang.llvm.org/extra/clang-tidy/checks/performance/noexcept-move-constructor.html
-class NoexceptMoveConstructorCheck : public ClangTidyCheck {
+class NoexceptMoveConstructorCheck : public BaseNoexceptFunctionCheck {
 public:
-  NoexceptMoveConstructorCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
-  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
-    return LangOpts.CPlusPlus11 && LangOpts.CXXExceptions;
-  }
+  using BaseNoexceptFunctionCheck::BaseNoexceptFunctionCheck;
+
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
-  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
-  std::optional<TraversalKind> getCheckTraversalKind() const override {
-    return TK_IgnoreUnlessSpelledInSource;
-  }
 
-private:
-  utils::ExceptionSpecAnalyzer SpecAnalyzer;
+protected:
+  DiagnosticBuilder
+  reportMissingNoexcept(const FunctionDecl *FuncDecl) final override;
+  void reportNoexceptEvaluatedToFalse(const FunctionDecl *FuncDecl,
+                                      const Expr *NoexceptExpr) final override;
 };
 
 } // namespace clang::tidy::performance
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTMOVECONSTRUCTORCHECK_H

clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp

 //===--- NoexceptMoveConstructorCheck.cpp - clang-tidy---------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "NoexceptMoveConstructorCheck.h"
-#include "../utils/LexerUtils.h"
-#include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/Lex/Lexer.h"
-#include "clang/Tooling/FixIt.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang::tidy::performance {
 
 void NoexceptMoveConstructorCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       cxxMethodDecl(unless(isDeleted()),
                     anyOf(cxxConstructorDecl(isMoveConstructor()),
                           isMoveAssignmentOperator()))
-          .bind("decl"),
+          .bind(BindFuncDeclName),
       this);
 }
 
-void NoexceptMoveConstructorCheck::check(
-    const MatchFinder::MatchResult &Result) {
-  const auto *FuncDecl = Result.Nodes.getNodeAs<CXXMethodDecl>("decl");
-  assert(FuncDecl);
-
-  if (SpecAnalyzer.analyze(FuncDecl) !=
-      utils::ExceptionSpecAnalyzer::State::Throwing)
-    return;
-
-  const bool IsConstructor = CXXConstructorDecl::classof(FuncDecl);
-
-  // Don't complain about nothrow(false), but complain on nothrow(expr)
-  // where expr evaluates to false.
-  const auto *ProtoType = FuncDecl->getType()->castAs<FunctionProtoType>();
-  const Expr *NoexceptExpr = ProtoType->getNoexceptExpr();
-  if (NoexceptExpr) {
-    NoexceptExpr = NoexceptExpr->IgnoreImplicit();
-    if (!isa<CXXBoolLiteralExpr>(NoexceptExpr)) {
-      diag(NoexceptExpr->getExprLoc(),
-           "noexcept specifier on the move %select{assignment "
-           "operator|constructor}0 evaluates to 'false'")
-          << IsConstructor;
-    }
-    return;
-  }
-
-  auto Diag = diag(FuncDecl->getLocation(),
-                   "move %select{assignment operator|constructor}0s should "
-                   "be marked noexcept")
-              << IsConstructor;
-  // Add FixIt hints.
-
-  const SourceManager &SM = *Result.SourceManager;
+DiagnosticBuilder NoexceptMoveConstructorCheck::reportMissingNoexcept(
+    const FunctionDecl *FuncDecl) {
+  return diag(FuncDecl->getLocation(),
+         << CXXConstructorDecl::classof(FuncDecl);
+}
 
-  const SourceLocation NoexceptLoc =
-      utils::lexer::getLocationForNoexceptSpecifier(FuncDecl, SM);
-  if (NoexceptLoc.isValid())
-    Diag << FixItHint::CreateInsertion(NoexceptLoc, " noexcept ");
+void NoexceptMoveConstructorCheck::reportNoexceptEvaluatedToFalse(
+    const FunctionDecl *FuncDecl, const Expr *NoexceptExpr) {
+  diag(NoexceptExpr->getExprLoc(),
+       "noexcept specifier on the move %select{assignment "
+       "operator|constructor}0 evaluates to 'false'")
+      << CXXConstructorDecl::classof(FuncDecl);
 }
 
 } // namespace clang::tidy::performance

clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.h

 //===--- NoexceptSwapCheck.h - clang-tidy -----------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTSWAPCHECK_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTSWAPCHECK_H
 
 #include "../ClangTidyCheck.h"
-#include "../utils/ExceptionSpecAnalyzer.h"
+#include "BaseNoexceptFunctionCheck.h"
 
 namespace clang::tidy::performance {
 
 /// The check flags swap functions not marked with `noexcept` or marked
 /// with `noexcept(expr)` where `expr` evaluates to `false`
 /// (but is not a `false` literal itself).
 ///
 /// For the user-facing documentation see:
 /// https://clang.llvm.org/extra/clang-tidy/checks/performance/noexcept-swap.html
-class NoexceptSwapCheck : public ClangTidyCheck {
+class NoexceptSwapCheck : public BaseNoexceptFunctionCheck {
 public:
-  NoexceptSwapCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  using BaseNoexceptFunctionCheck::BaseNoexceptFunctionCheck;
+
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
-  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
-    return LangOpts.CPlusPlus11 && LangOpts.CXXExceptions;
-  }
-  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
-  std::optional<TraversalKind> getCheckTraversalKind() const override {
-    return TK_IgnoreUnlessSpelledInSource;
-  }
 
-private:
-  utils::ExceptionSpecAnalyzer SpecAnalyzer;
+protected:
+  DiagnosticBuilder
+  reportMissingNoexcept(const FunctionDecl *FuncDecl) final override;
+  void reportNoexceptEvaluatedToFalse(const FunctionDecl *FuncDecl,
+                                      const Expr *NoexceptExpr) final override;
 };
 
 } // namespace clang::tidy::performance
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTSWAPCHECK_H

clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp

 //===--- NoexceptSwapCheck.cpp - clang-tidy -------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "NoexceptSwapCheck.h"
-#include "../utils/LexerUtils.h"
-#include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang::tidy::performance {
 
 void NoexceptSwapCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
-      functionDecl(unless(isDeleted()), hasName("swap")).bind("decl"), this);
+      functionDecl(unless(isDeleted()), hasName("swap")).bind(BindFuncDeclName),
+      this);
 }
 
-void NoexceptSwapCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>("decl");
-  assert(FuncDecl);
-
-  if (SpecAnalyzer.analyze(FuncDecl) !=
-      utils::ExceptionSpecAnalyzer::State::Throwing)
-    return;
-
-  // Don't complain about nothrow(false), but complain on nothrow(expr)
-  // where expr evaluates to false.
-  const auto *ProtoType = FuncDecl->getType()->castAs<FunctionProtoType>();
-  const Expr *NoexceptExpr = ProtoType->getNoexceptExpr();
-  if (NoexceptExpr) {
-    NoexceptExpr = NoexceptExpr->IgnoreImplicit();
-    if (!isa<CXXBoolLiteralExpr>(NoexceptExpr)) {
-      diag(NoexceptExpr->getExprLoc(),
-           "noexcept specifier on swap function evaluates to 'false'");
-    }
-    return;
-  }
-
-  auto Diag = diag(FuncDecl->getLocation(), "swap functions should "
-                                            "be marked noexcept");
+DiagnosticBuilder
+NoexceptSwapCheck::reportMissingNoexcept(const FunctionDecl *FuncDecl) {
+  return diag(FuncDecl->getLocation(), "swap functions should "
+}
 
-  // Add FixIt hints.
-  const SourceManager &SM = *Result.SourceManager;
-
-  const SourceLocation NoexceptLoc =
-      utils::lexer::getLocationForNoexceptSpecifier(FuncDecl, SM);
-  if (NoexceptLoc.isValid())
-    Diag << FixItHint::CreateInsertion(NoexceptLoc, " noexcept ");
+void NoexceptSwapCheck::reportNoexceptEvaluatedToFalse(
+    const FunctionDecl *FuncDecl, const Expr *NoexceptExpr) {
+  diag(NoexceptExpr->getExprLoc(),
+       "noexcept specifier on swap function evaluates to 'false'");
 }
 
 } // namespace clang::tidy::performance