[clang-tidy] Refactor common code from the Noexcept*Checks into `NoexceptFunctionCheck`
authorAMS21 <AMS21.github@gmail.com>
Sun, 18 Jun 2023 06:50:05 +0000 (06:50 +0000)
committerPiotr Zegar <me@piotrzegar.pl>
Sun, 18 Jun 2023 06:50:05 +0000 (06:50 +0000)
As discussed in the https://reviews.llvm.org/D148697 review.

Reviewed By: PiotrZSL

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

clang-tools-extra/clang-tidy/performance/CMakeLists.txt
clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.cpp
clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.h
clang-tools-extra/clang-tidy/performance/NoexceptFunctionBaseCheck.cpp [new file with mode: 0644]
clang-tools-extra/clang-tidy/performance/NoexceptFunctionBaseCheck.h [new file with mode: 0644]
clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.h
clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp
clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.h

index 74a2cac..de13894 100644 (file)
@@ -16,6 +16,7 @@ add_clang_library(clangTidyPerformanceModule
   NoAutomaticMoveCheck.cpp
   NoIntToPtrCheck.cpp
   NoexceptDestructorCheck.cpp
+  NoexceptFunctionBaseCheck.cpp
   NoexceptMoveConstructorCheck.cpp
   NoexceptSwapCheck.cpp
   PerformanceTidyModule.cpp
index a21451d..9f28b8e 100644 (file)
@@ -7,8 +7,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "NoexceptDestructorCheck.h"
-#include "../utils/LexerUtils.h"
-#include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 
 using namespace clang::ast_matchers;
@@ -16,42 +14,21 @@ using namespace clang::ast_matchers;
 namespace clang::tidy::performance {
 
 void NoexceptDestructorCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(
-      functionDecl(unless(isDeleted()), cxxDestructorDecl()).bind("decl"),
-      this);
+  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");
-
-  // Add FixIt hints.
-  const SourceManager &SM = *Result.SourceManager;
+DiagnosticBuilder
+NoexceptDestructorCheck::reportMissingNoexcept(const FunctionDecl *FuncDecl) {
+  return diag(FuncDecl->getLocation(), "destructors should "
+                                       "be marked noexcept");
+}
 
-  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
index 38ae927..ab3850f 100644 (file)
@@ -10,7 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTDESTRUCTORCHECK_H
 
 #include "../ClangTidyCheck.h"
-#include "../utils/ExceptionSpecAnalyzer.h"
+#include "NoexceptFunctionBaseCheck.h"
 
 namespace clang::tidy::performance {
 
@@ -20,21 +20,17 @@ namespace clang::tidy::performance {
 ///
 /// 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 NoexceptFunctionBaseCheck {
 public:
-  NoexceptDestructorCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  using NoexceptFunctionBaseCheck::NoexceptFunctionBaseCheck;
+
   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;
+  DiagnosticBuilder
+  reportMissingNoexcept(const FunctionDecl *FuncDecl) final override;
+  void reportNoexceptEvaluatedToFalse(const FunctionDecl *FuncDecl,
+                                      const Expr *NoexceptExpr) final override;
 };
 
 } // namespace clang::tidy::performance
diff --git a/clang-tools-extra/clang-tidy/performance/NoexceptFunctionBaseCheck.cpp b/clang-tools-extra/clang-tidy/performance/NoexceptFunctionBaseCheck.cpp
new file mode 100644 (file)
index 0000000..911cd1b
--- /dev/null
@@ -0,0 +1,49 @@
+//===--- NoexceptFunctionCheck.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 "NoexceptFunctionBaseCheck.h"
+#include "../utils/LexerUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::performance {
+
+void NoexceptFunctionBaseCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>(BindFuncDeclName);
+  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))
+      reportNoexceptEvaluatedToFalse(FuncDecl, NoexceptExpr);
+    return;
+  }
+
+  auto Diag = reportMissingNoexcept(FuncDecl);
+
+  // Add FixIt hints.
+  const SourceManager &SM = *Result.SourceManager;
+
+  const SourceLocation NoexceptLoc =
+      utils::lexer::getLocationForNoexceptSpecifier(FuncDecl, SM);
+  if (NoexceptLoc.isValid())
+    Diag << FixItHint::CreateInsertion(NoexceptLoc, " noexcept ");
+}
+
+} // namespace clang::tidy::performance
diff --git a/clang-tools-extra/clang-tidy/performance/NoexceptFunctionBaseCheck.h b/clang-tools-extra/clang-tidy/performance/NoexceptFunctionBaseCheck.h
new file mode 100644 (file)
index 0000000..4775219
--- /dev/null
@@ -0,0 +1,50 @@
+//===--- NoexceptFunctionCheck.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_NOEXCEPTFUNCTIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTFUNCTIONCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/ExceptionSpecAnalyzer.h"
+#include "clang/AST/Decl.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace clang::tidy::performance {
+
+/// Generic check which checks if the bound function decl is
+/// marked with `noexcept` or `noexcept(expr)` where `expr` evaluates to
+/// `false`.
+class NoexceptFunctionBaseCheck : public ClangTidyCheck {
+public:
+  NoexceptFunctionBaseCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus11 && LangOpts.CXXExceptions;
+  }
+  void
+  check(const ast_matchers::MatchFinder::MatchResult &Result) final override;
+  std::optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
+
+protected:
+  virtual DiagnosticBuilder
+  reportMissingNoexcept(const FunctionDecl *FuncDecl) = 0;
+  virtual void reportNoexceptEvaluatedToFalse(const FunctionDecl *FuncDecl,
+                                              const Expr *NoexceptExpr) = 0;
+
+  static constexpr StringRef BindFuncDeclName = "FuncDecl";
+
+private:
+  utils::ExceptionSpecAnalyzer SpecAnalyzer;
+};
+
+} // namespace clang::tidy::performance
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTFUNCTIONCHECK_H
index b56a99a..83b33d5 100644 (file)
@@ -7,11 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #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;
 
@@ -22,48 +18,24 @@ void NoexceptMoveConstructorCheck::registerMatchers(MatchFinder *Finder) {
       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(),
+              "move %select{assignment operator|constructor}0s should "
+              "be marked noexcept")
+         << 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
index 053e6d9..51728d2 100644 (file)
@@ -10,7 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTMOVECONSTRUCTORCHECK_H
 
 #include "../ClangTidyCheck.h"
-#include "../utils/ExceptionSpecAnalyzer.h"
+#include "NoexceptFunctionBaseCheck.h"
 
 namespace clang::tidy::performance {
 
@@ -24,21 +24,17 @@ namespace clang::tidy::performance {
 ///
 /// 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 NoexceptFunctionBaseCheck {
 public:
-  NoexceptMoveConstructorCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
-  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
-    return LangOpts.CPlusPlus11 && LangOpts.CXXExceptions;
-  }
+  using NoexceptFunctionBaseCheck::NoexceptFunctionBaseCheck;
+
   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;
+  DiagnosticBuilder
+  reportMissingNoexcept(const FunctionDecl *FuncDecl) final override;
+  void reportNoexceptEvaluatedToFalse(const FunctionDecl *FuncDecl,
+                                      const Expr *NoexceptExpr) final override;
 };
 
 } // namespace clang::tidy::performance
index 170a918..67c598e 100644 (file)
@@ -7,10 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #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;
 
@@ -18,40 +15,20 @@ 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");
-
-  // Add FixIt hints.
-  const SourceManager &SM = *Result.SourceManager;
+DiagnosticBuilder
+NoexceptSwapCheck::reportMissingNoexcept(const FunctionDecl *FuncDecl) {
+  return diag(FuncDecl->getLocation(), "swap functions should "
+                                       "be marked noexcept");
+}
 
-  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
index 5578aec..0330de4 100644 (file)
@@ -10,7 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTSWAPCHECK_H
 
 #include "../ClangTidyCheck.h"
-#include "../utils/ExceptionSpecAnalyzer.h"
+#include "NoexceptFunctionBaseCheck.h"
 
 namespace clang::tidy::performance {
 
@@ -20,21 +20,17 @@ namespace clang::tidy::performance {
 ///
 /// 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 NoexceptFunctionBaseCheck {
 public:
-  NoexceptSwapCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  using NoexceptFunctionBaseCheck::NoexceptFunctionBaseCheck;
+
   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;
+  DiagnosticBuilder
+  reportMissingNoexcept(const FunctionDecl *FuncDecl) final override;
+  void reportNoexceptEvaluatedToFalse(const FunctionDecl *FuncDecl,
+                                      const Expr *NoexceptExpr) final override;
 };
 
 } // namespace clang::tidy::performance