From 845618cf69e89313084a4e93f8ff31d8e6ea4499 Mon Sep 17 00:00:00 2001 From: AMS21 Date: Sun, 18 Jun 2023 06:50:05 +0000 Subject: [PATCH] [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 --- .../clang-tidy/performance/CMakeLists.txt | 1 + .../performance/NoexceptDestructorCheck.cpp | 47 +++++------------- .../performance/NoexceptDestructorCheck.h | 20 ++++---- .../performance/NoexceptFunctionBaseCheck.cpp | 49 +++++++++++++++++++ .../performance/NoexceptFunctionBaseCheck.h | 50 +++++++++++++++++++ .../performance/NoexceptMoveConstructorCheck.cpp | 56 ++++++---------------- .../performance/NoexceptMoveConstructorCheck.h | 20 ++++---- .../clang-tidy/performance/NoexceptSwapCheck.cpp | 45 +++++------------ .../clang-tidy/performance/NoexceptSwapCheck.h | 20 ++++---- 9 files changed, 161 insertions(+), 147 deletions(-) create mode 100644 clang-tools-extra/clang-tidy/performance/NoexceptFunctionBaseCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/performance/NoexceptFunctionBaseCheck.h diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt index 74a2cac..de13894 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 a21451d..9f28b8e 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("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(); - const Expr *NoexceptExpr = ProtoType->getNoexceptExpr(); - if (NoexceptExpr) { - NoexceptExpr = NoexceptExpr->IgnoreImplicit(); - if (!isa(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 38ae927..ab3850f 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 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 0000000..911cd1b --- /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(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(); + const Expr *NoexceptExpr = ProtoType->getNoexceptExpr(); + if (NoexceptExpr) { + NoexceptExpr = NoexceptExpr->IgnoreImplicit(); + if (!isa(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 0000000..4775219 --- /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 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 b56a99a..83b33d5 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("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(); - const Expr *NoexceptExpr = ProtoType->getNoexceptExpr(); - if (NoexceptExpr) { - NoexceptExpr = NoexceptExpr->IgnoreImplicit(); - if (!isa(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 053e6d9..51728d2 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 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 170a918..67c598e 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("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(); - const Expr *NoexceptExpr = ProtoType->getNoexceptExpr(); - if (NoexceptExpr) { - NoexceptExpr = NoexceptExpr->IgnoreImplicit(); - if (!isa(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 5578aec..0330de4 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 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 -- 2.7.4