Skip to content

Commit 845618c

Browse files
AMS21PiotrZSL
AMS21
authored and
PiotrZSL
committed
[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
1 parent 7400bdc commit 845618c

9 files changed

+161
-147
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ add_clang_library(clangTidyPerformanceModule
1616
NoAutomaticMoveCheck.cpp
1717
NoIntToPtrCheck.cpp
1818
NoexceptDestructorCheck.cpp
19+
NoexceptFunctionBaseCheck.cpp
1920
NoexceptMoveConstructorCheck.cpp
2021
NoexceptSwapCheck.cpp
2122
PerformanceTidyModule.cpp

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

Lines changed: 12 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -7,51 +7,28 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "NoexceptDestructorCheck.h"
10-
#include "../utils/LexerUtils.h"
11-
#include "clang/AST/ASTContext.h"
1210
#include "clang/ASTMatchers/ASTMatchFinder.h"
1311

1412
using namespace clang::ast_matchers;
1513

1614
namespace clang::tidy::performance {
1715

1816
void NoexceptDestructorCheck::registerMatchers(MatchFinder *Finder) {
19-
Finder->addMatcher(
20-
functionDecl(unless(isDeleted()), cxxDestructorDecl()).bind("decl"),
21-
this);
17+
Finder->addMatcher(functionDecl(unless(isDeleted()), cxxDestructorDecl())
18+
.bind(BindFuncDeclName),
19+
this);
2220
}
2321

24-
void NoexceptDestructorCheck::check(const MatchFinder::MatchResult &Result) {
25-
const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>("decl");
26-
assert(FuncDecl);
27-
28-
if (SpecAnalyzer.analyze(FuncDecl) !=
29-
utils::ExceptionSpecAnalyzer::State::Throwing)
30-
return;
31-
32-
// Don't complain about nothrow(false), but complain on nothrow(expr)
33-
// where expr evaluates to false.
34-
const auto *ProtoType = FuncDecl->getType()->castAs<FunctionProtoType>();
35-
const Expr *NoexceptExpr = ProtoType->getNoexceptExpr();
36-
if (NoexceptExpr) {
37-
NoexceptExpr = NoexceptExpr->IgnoreImplicit();
38-
if (!isa<CXXBoolLiteralExpr>(NoexceptExpr)) {
39-
diag(NoexceptExpr->getExprLoc(),
40-
"noexcept specifier on the destructor evaluates to 'false'");
41-
}
42-
return;
43-
}
44-
45-
auto Diag = diag(FuncDecl->getLocation(), "destructors should "
46-
"be marked noexcept");
47-
48-
// Add FixIt hints.
49-
const SourceManager &SM = *Result.SourceManager;
22+
DiagnosticBuilder
23+
NoexceptDestructorCheck::reportMissingNoexcept(const FunctionDecl *FuncDecl) {
24+
return diag(FuncDecl->getLocation(), "destructors should "
25+
"be marked noexcept");
26+
}
5027

51-
const SourceLocation NoexceptLoc =
52-
utils::lexer::getLocationForNoexceptSpecifier(FuncDecl, SM);
53-
if (NoexceptLoc.isValid())
54-
Diag << FixItHint::CreateInsertion(NoexceptLoc, " noexcept ");
28+
void NoexceptDestructorCheck::reportNoexceptEvaluatedToFalse(
29+
const FunctionDecl *FuncDecl, const Expr *NoexceptExpr) {
30+
diag(NoexceptExpr->getExprLoc(),
31+
"noexcept specifier on the destructor evaluates to 'false'");
5532
}
5633

5734
} // namespace clang::tidy::performance

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

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTDESTRUCTORCHECK_H
1111

1212
#include "../ClangTidyCheck.h"
13-
#include "../utils/ExceptionSpecAnalyzer.h"
13+
#include "NoexceptFunctionBaseCheck.h"
1414

1515
namespace clang::tidy::performance {
1616

@@ -20,21 +20,17 @@ namespace clang::tidy::performance {
2020
///
2121
/// For the user-facing documentation see:
2222
/// https://clang.llvm.org/extra/clang-tidy/checks/performance/noexcept-destructor.html
23-
class NoexceptDestructorCheck : public ClangTidyCheck {
23+
class NoexceptDestructorCheck : public NoexceptFunctionBaseCheck {
2424
public:
25-
NoexceptDestructorCheck(StringRef Name, ClangTidyContext *Context)
26-
: ClangTidyCheck(Name, Context) {}
25+
using NoexceptFunctionBaseCheck::NoexceptFunctionBaseCheck;
26+
2727
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
28-
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
29-
return LangOpts.CPlusPlus11 && LangOpts.CXXExceptions;
30-
}
31-
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
32-
std::optional<TraversalKind> getCheckTraversalKind() const override {
33-
return TK_IgnoreUnlessSpelledInSource;
34-
}
3528

3629
private:
37-
utils::ExceptionSpecAnalyzer SpecAnalyzer;
30+
DiagnosticBuilder
31+
reportMissingNoexcept(const FunctionDecl *FuncDecl) final override;
32+
void reportNoexceptEvaluatedToFalse(const FunctionDecl *FuncDecl,
33+
const Expr *NoexceptExpr) final override;
3834
};
3935

4036
} // namespace clang::tidy::performance

‎clang-tools-extra/clang-tidy/performance/NoexceptFunctionBaseCheck.cpp

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
//===--- NoexceptFunctionCheck.cpp - clang-tidy ---------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "NoexceptFunctionBaseCheck.h"
10+
#include "../utils/LexerUtils.h"
11+
#include "clang/AST/ASTContext.h"
12+
#include "clang/AST/Decl.h"
13+
#include "clang/ASTMatchers/ASTMatchFinder.h"
14+
15+
using namespace clang::ast_matchers;
16+
17+
namespace clang::tidy::performance {
18+
19+
void NoexceptFunctionBaseCheck::check(const MatchFinder::MatchResult &Result) {
20+
const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>(BindFuncDeclName);
21+
assert(FuncDecl);
22+
23+
if (SpecAnalyzer.analyze(FuncDecl) !=
24+
utils::ExceptionSpecAnalyzer::State::Throwing)
25+
return;
26+
27+
// Don't complain about nothrow(false), but complain on nothrow(expr)
28+
// where expr evaluates to false.
29+
const auto *ProtoType = FuncDecl->getType()->castAs<FunctionProtoType>();
30+
const Expr *NoexceptExpr = ProtoType->getNoexceptExpr();
31+
if (NoexceptExpr) {
32+
NoexceptExpr = NoexceptExpr->IgnoreImplicit();
33+
if (!isa<CXXBoolLiteralExpr>(NoexceptExpr))
34+
reportNoexceptEvaluatedToFalse(FuncDecl, NoexceptExpr);
35+
return;
36+
}
37+
38+
auto Diag = reportMissingNoexcept(FuncDecl);
39+
40+
// Add FixIt hints.
41+
const SourceManager &SM = *Result.SourceManager;
42+
43+
const SourceLocation NoexceptLoc =
44+
utils::lexer::getLocationForNoexceptSpecifier(FuncDecl, SM);
45+
if (NoexceptLoc.isValid())
46+
Diag << FixItHint::CreateInsertion(NoexceptLoc, " noexcept ");
47+
}
48+
49+
} // namespace clang::tidy::performance

‎clang-tools-extra/clang-tidy/performance/NoexceptFunctionBaseCheck.h

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
//===--- NoexceptFunctionCheck.h - clang-tidy -------------------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTFUNCTIONCHECK_H
10+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTFUNCTIONCHECK_H
11+
12+
#include "../ClangTidyCheck.h"
13+
#include "../utils/ExceptionSpecAnalyzer.h"
14+
#include "clang/AST/Decl.h"
15+
#include "llvm/ADT/StringRef.h"
16+
17+
namespace clang::tidy::performance {
18+
19+
/// Generic check which checks if the bound function decl is
20+
/// marked with `noexcept` or `noexcept(expr)` where `expr` evaluates to
21+
/// `false`.
22+
class NoexceptFunctionBaseCheck : public ClangTidyCheck {
23+
public:
24+
NoexceptFunctionBaseCheck(StringRef Name, ClangTidyContext *Context)
25+
: ClangTidyCheck(Name, Context) {}
26+
27+
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
28+
return LangOpts.CPlusPlus11 && LangOpts.CXXExceptions;
29+
}
30+
void
31+
check(const ast_matchers::MatchFinder::MatchResult &Result) final override;
32+
std::optional<TraversalKind> getCheckTraversalKind() const override {
33+
return TK_IgnoreUnlessSpelledInSource;
34+
}
35+
36+
protected:
37+
virtual DiagnosticBuilder
38+
reportMissingNoexcept(const FunctionDecl *FuncDecl) = 0;
39+
virtual void reportNoexceptEvaluatedToFalse(const FunctionDecl *FuncDecl,
40+
const Expr *NoexceptExpr) = 0;
41+
42+
static constexpr StringRef BindFuncDeclName = "FuncDecl";
43+
44+
private:
45+
utils::ExceptionSpecAnalyzer SpecAnalyzer;
46+
};
47+
48+
} // namespace clang::tidy::performance
49+
50+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTFUNCTIONCHECK_H

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

Lines changed: 14 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,7 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "NoexceptMoveConstructorCheck.h"
10-
#include "../utils/LexerUtils.h"
11-
#include "clang/AST/ASTContext.h"
1210
#include "clang/ASTMatchers/ASTMatchFinder.h"
13-
#include "clang/Lex/Lexer.h"
14-
#include "clang/Tooling/FixIt.h"
1511

1612
using namespace clang::ast_matchers;
1713

@@ -22,48 +18,24 @@ void NoexceptMoveConstructorCheck::registerMatchers(MatchFinder *Finder) {
2218
cxxMethodDecl(unless(isDeleted()),
2319
anyOf(cxxConstructorDecl(isMoveConstructor()),
2420
isMoveAssignmentOperator()))
25-
.bind("decl"),
21+
.bind(BindFuncDeclName),
2622
this);
2723
}
2824

29-
void NoexceptMoveConstructorCheck::check(
30-
const MatchFinder::MatchResult &Result) {
31-
const auto *FuncDecl = Result.Nodes.getNodeAs<CXXMethodDecl>("decl");
32-
assert(FuncDecl);
33-
34-
if (SpecAnalyzer.analyze(FuncDecl) !=
35-
utils::ExceptionSpecAnalyzer::State::Throwing)
36-
return;
37-
38-
const bool IsConstructor = CXXConstructorDecl::classof(FuncDecl);
39-
40-
// Don't complain about nothrow(false), but complain on nothrow(expr)
41-
// where expr evaluates to false.
42-
const auto *ProtoType = FuncDecl->getType()->castAs<FunctionProtoType>();
43-
const Expr *NoexceptExpr = ProtoType->getNoexceptExpr();
44-
if (NoexceptExpr) {
45-
NoexceptExpr = NoexceptExpr->IgnoreImplicit();
46-
if (!isa<CXXBoolLiteralExpr>(NoexceptExpr)) {
47-
diag(NoexceptExpr->getExprLoc(),
48-
"noexcept specifier on the move %select{assignment "
49-
"operator|constructor}0 evaluates to 'false'")
50-
<< IsConstructor;
51-
}
52-
return;
53-
}
54-
55-
auto Diag = diag(FuncDecl->getLocation(),
56-
"move %select{assignment operator|constructor}0s should "
57-
"be marked noexcept")
58-
<< IsConstructor;
59-
// Add FixIt hints.
60-
61-
const SourceManager &SM = *Result.SourceManager;
25+
DiagnosticBuilder NoexceptMoveConstructorCheck::reportMissingNoexcept(
26+
const FunctionDecl *FuncDecl) {
27+
return diag(FuncDecl->getLocation(),
28+
"move %select{assignment operator|constructor}0s should "
29+
"be marked noexcept")
30+
<< CXXConstructorDecl::classof(FuncDecl);
31+
}
6232

63-
const SourceLocation NoexceptLoc =
64-
utils::lexer::getLocationForNoexceptSpecifier(FuncDecl, SM);
65-
if (NoexceptLoc.isValid())
66-
Diag << FixItHint::CreateInsertion(NoexceptLoc, " noexcept ");
33+
void NoexceptMoveConstructorCheck::reportNoexceptEvaluatedToFalse(
34+
const FunctionDecl *FuncDecl, const Expr *NoexceptExpr) {
35+
diag(NoexceptExpr->getExprLoc(),
36+
"noexcept specifier on the move %select{assignment "
37+
"operator|constructor}0 evaluates to 'false'")
38+
<< CXXConstructorDecl::classof(FuncDecl);
6739
}
6840

6941
} // namespace clang::tidy::performance

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

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTMOVECONSTRUCTORCHECK_H
1111

1212
#include "../ClangTidyCheck.h"
13-
#include "../utils/ExceptionSpecAnalyzer.h"
13+
#include "NoexceptFunctionBaseCheck.h"
1414

1515
namespace clang::tidy::performance {
1616

@@ -24,21 +24,17 @@ namespace clang::tidy::performance {
2424
///
2525
/// For the user-facing documentation see:
2626
/// https://clang.llvm.org/extra/clang-tidy/checks/performance/noexcept-move-constructor.html
27-
class NoexceptMoveConstructorCheck : public ClangTidyCheck {
27+
class NoexceptMoveConstructorCheck : public NoexceptFunctionBaseCheck {
2828
public:
29-
NoexceptMoveConstructorCheck(StringRef Name, ClangTidyContext *Context)
30-
: ClangTidyCheck(Name, Context) {}
31-
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
32-
return LangOpts.CPlusPlus11 && LangOpts.CXXExceptions;
33-
}
29+
using NoexceptFunctionBaseCheck::NoexceptFunctionBaseCheck;
30+
3431
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
35-
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
36-
std::optional<TraversalKind> getCheckTraversalKind() const override {
37-
return TK_IgnoreUnlessSpelledInSource;
38-
}
3932

4033
private:
41-
utils::ExceptionSpecAnalyzer SpecAnalyzer;
34+
DiagnosticBuilder
35+
reportMissingNoexcept(const FunctionDecl *FuncDecl) final override;
36+
void reportNoexceptEvaluatedToFalse(const FunctionDecl *FuncDecl,
37+
const Expr *NoexceptExpr) final override;
4238
};
4339

4440
} // namespace clang::tidy::performance

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

Lines changed: 11 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -7,51 +7,28 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "NoexceptSwapCheck.h"
10-
#include "../utils/LexerUtils.h"
11-
#include "clang/AST/ASTContext.h"
1210
#include "clang/ASTMatchers/ASTMatchFinder.h"
13-
#include "clang/Lex/Lexer.h"
1411

1512
using namespace clang::ast_matchers;
1613

1714
namespace clang::tidy::performance {
1815

1916
void NoexceptSwapCheck::registerMatchers(MatchFinder *Finder) {
2017
Finder->addMatcher(
21-
functionDecl(unless(isDeleted()), hasName("swap")).bind("decl"), this);
18+
functionDecl(unless(isDeleted()), hasName("swap")).bind(BindFuncDeclName),
19+
this);
2220
}
2321

24-
void NoexceptSwapCheck::check(const MatchFinder::MatchResult &Result) {
25-
const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>("decl");
26-
assert(FuncDecl);
27-
28-
if (SpecAnalyzer.analyze(FuncDecl) !=
29-
utils::ExceptionSpecAnalyzer::State::Throwing)
30-
return;
31-
32-
// Don't complain about nothrow(false), but complain on nothrow(expr)
33-
// where expr evaluates to false.
34-
const auto *ProtoType = FuncDecl->getType()->castAs<FunctionProtoType>();
35-
const Expr *NoexceptExpr = ProtoType->getNoexceptExpr();
36-
if (NoexceptExpr) {
37-
NoexceptExpr = NoexceptExpr->IgnoreImplicit();
38-
if (!isa<CXXBoolLiteralExpr>(NoexceptExpr)) {
39-
diag(NoexceptExpr->getExprLoc(),
40-
"noexcept specifier on swap function evaluates to 'false'");
41-
}
42-
return;
43-
}
44-
45-
auto Diag = diag(FuncDecl->getLocation(), "swap functions should "
46-
"be marked noexcept");
47-
48-
// Add FixIt hints.
49-
const SourceManager &SM = *Result.SourceManager;
22+
DiagnosticBuilder
23+
NoexceptSwapCheck::reportMissingNoexcept(const FunctionDecl *FuncDecl) {
24+
return diag(FuncDecl->getLocation(), "swap functions should "
25+
"be marked noexcept");
26+
}
5027

51-
const SourceLocation NoexceptLoc =
52-
utils::lexer::getLocationForNoexceptSpecifier(FuncDecl, SM);
53-
if (NoexceptLoc.isValid())
54-
Diag << FixItHint::CreateInsertion(NoexceptLoc, " noexcept ");
28+
void NoexceptSwapCheck::reportNoexceptEvaluatedToFalse(
29+
const FunctionDecl *FuncDecl, const Expr *NoexceptExpr) {
30+
diag(NoexceptExpr->getExprLoc(),
31+
"noexcept specifier on swap function evaluates to 'false'");
5532
}
5633

5734
} // namespace clang::tidy::performance

0 commit comments

Comments
 (0)