[clang-tools-extra] 845618c - [clang-tidy] Refactor common code from the Noexcept*Checks into `NoexceptFunctionCheck`
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Sun Jun 18 00:30:21 PDT 2023
Author: AMS21
Date: 2023-06-18T06:50:05Z
New Revision: 845618cf69e89313084a4e93f8ff31d8e6ea4499
URL: https://github.com/llvm/llvm-project/commit/845618cf69e89313084a4e93f8ff31d8e6ea4499
DIFF: https://github.com/llvm/llvm-project/commit/845618cf69e89313084a4e93f8ff31d8e6ea4499.diff
LOG: [clang-tidy] Refactor common code from the Noexcept*Checks into `NoexceptFunctionCheck`
As discussed in the https://reviews.llvm.org/D148697 review.
Reviewed By: PiotrZSL
Differential Revision: https://reviews.llvm.org/D153198
Added:
clang-tools-extra/clang-tidy/performance/NoexceptFunctionBaseCheck.cpp
clang-tools-extra/clang-tidy/performance/NoexceptFunctionBaseCheck.h
Modified:
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/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
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
index 74a2cac092f86..de13894dabc34 100644
--- a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
@@ -16,6 +16,7 @@ add_clang_library(clangTidyPerformanceModule
NoAutomaticMoveCheck.cpp
NoIntToPtrCheck.cpp
NoexceptDestructorCheck.cpp
+ NoexceptFunctionBaseCheck.cpp
NoexceptMoveConstructorCheck.cpp
NoexceptSwapCheck.cpp
PerformanceTidyModule.cpp
diff --git a/clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.cpp b/clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.cpp
index a21451dd232df..9f28b8ef30876 100644
--- a/clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.cpp
@@ -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
diff --git a/clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.h b/clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.h
index 38ae9272b7de7..ab3850f0970a8 100644
--- a/clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.h
+++ b/clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.h
@@ -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
index 0000000000000..911cd1b533367
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/NoexceptFunctionBaseCheck.cpp
@@ -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
index 0000000000000..4775219d7e439
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/NoexceptFunctionBaseCheck.h
@@ -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
diff --git a/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp b/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
index b56a99a8f3491..83b33d53ab553 100644
--- a/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
@@ -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
diff --git a/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.h b/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.h
index 053e6d992eeef..51728d2ce0d8d 100644
--- a/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.h
+++ b/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.h
@@ -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
diff --git a/clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp b/clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp
index 170a918b3e575..67c598e0134cc 100644
--- a/clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp
@@ -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
diff --git a/clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.h b/clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.h
index 5578aec986d2b..0330de4a50b43 100644
--- a/clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.h
+++ b/clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.h
@@ -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
More information about the cfe-commits
mailing list