AMS21 updated this revision to Diff 532414. AMS21 marked an inline comment as done. AMS21 added a comment.
So somehow I didn't add the actual `BaseNoexceptFunctionCheck` files, thats why the build failed. Also implemented the two new suggestions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153198/new/ https://reviews.llvm.org/D153198 Files: 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 clang-tools-extra/clang-tidy/performance/NoexceptFunctionBaseCheck.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
Index: clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.h =================================================================== --- clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.h +++ 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 @@ /// /// 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 Index: clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp +++ 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 @@ 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: clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.h =================================================================== --- clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.h +++ 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 @@ /// /// 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: clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp +++ 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 @@ 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: clang-tools-extra/clang-tidy/performance/NoexceptFunctionBaseCheck.h =================================================================== --- /dev/null +++ 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 Index: clang-tools-extra/clang-tidy/performance/NoexceptFunctionBaseCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/performance/NoexceptFunctionBaseCheck.cpp +++ clang-tools-extra/clang-tidy/performance/NoexceptFunctionBaseCheck.cpp @@ -1,4 +1,4 @@ -//===--- NoexceptDestructorCheck.cpp - clang-tidy -------------------------===// +//===--- 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. @@ -6,23 +6,18 @@ // //===----------------------------------------------------------------------===// -#include "NoexceptDestructorCheck.h" +#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 NoexceptDestructorCheck::registerMatchers(MatchFinder *Finder) { - Finder->addMatcher( - functionDecl(unless(isDeleted()), cxxDestructorDecl()).bind("decl"), - this); -} - -void NoexceptDestructorCheck::check(const MatchFinder::MatchResult &Result) { - const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>("decl"); +void NoexceptFunctionBaseCheck::check(const MatchFinder::MatchResult &Result) { + const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>(BindFuncDeclName); assert(FuncDecl); if (SpecAnalyzer.analyze(FuncDecl) != @@ -35,15 +30,12 @@ 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'"); - } + if (!isa<CXXBoolLiteralExpr>(NoexceptExpr)) + reportNoexceptEvaluatedToFalse(FuncDecl, NoexceptExpr); return; } - auto Diag = diag(FuncDecl->getLocation(), "destructors should " - "be marked noexcept"); + auto Diag = reportMissingNoexcept(FuncDecl); // Add FixIt hints. const SourceManager &SM = *Result.SourceManager; Index: clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.h =================================================================== --- clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.h +++ 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 @@ /// /// 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 Index: clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.cpp +++ 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 @@ 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: clang-tools-extra/clang-tidy/performance/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/performance/CMakeLists.txt +++ clang-tools-extra/clang-tidy/performance/CMakeLists.txt @@ -16,6 +16,7 @@ NoAutomaticMoveCheck.cpp NoIntToPtrCheck.cpp NoexceptDestructorCheck.cpp + NoexceptFunctionBaseCheck.cpp NoexceptMoveConstructorCheck.cpp NoexceptSwapCheck.cpp PerformanceTidyModule.cpp
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits