Skip to content

Commit

Permalink
[clang-tidy] Add more checks for functions which should be noexcept
Browse files Browse the repository at this point in the history
Added new checks
- `performance-noexcept-destructor`
- `performance-noexcept-swap`

Also added cppcoreguidlines aliases for the 2 new checks as well as `performance-noexcept-move-constructor`

This fixes #62154

Reviewed By: PiotrZSL

Differential Revision: https://reviews.llvm.org/D148697
  • Loading branch information
AMS21 authored and PiotrZSL committed Jun 13, 2023
1 parent 6eb6ee3 commit 474a2b9
Show file tree
Hide file tree
Showing 22 changed files with 945 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
#include "../modernize/AvoidCArraysCheck.h"
#include "../modernize/UseDefaultMemberInitCheck.h"
#include "../modernize/UseOverrideCheck.h"
#include "../performance/NoexceptDestructorCheck.h"
#include "../performance/NoexceptMoveConstructorCheck.h"
#include "../performance/NoexceptSwapCheck.h"
#include "../readability/MagicNumbersCheck.h"
#include "AvoidCapturingLambdaCoroutinesCheck.h"
#include "AvoidConstOrRefDataMembersCheck.h"
Expand Down Expand Up @@ -83,6 +86,12 @@ class CppCoreGuidelinesModule : public ClangTidyModule {
CheckFactories.registerCheck<NarrowingConversionsCheck>(
"cppcoreguidelines-narrowing-conversions");
CheckFactories.registerCheck<NoMallocCheck>("cppcoreguidelines-no-malloc");
CheckFactories.registerCheck<performance::NoexceptDestructorCheck>(
"cppcoreguidelines-noexcept-destructor");
CheckFactories.registerCheck<performance::NoexceptMoveConstructorCheck>(
"cppcoreguidelines-noexcept-move-operations");
CheckFactories.registerCheck<performance::NoexceptSwapCheck>(
"cppcoreguidelines-noexcept-swap");
CheckFactories.registerCheck<misc::NonPrivateMemberVariablesInClassesCheck>(
"cppcoreguidelines-non-private-member-variables-in-classes");
CheckFactories.registerCheck<OwningMemoryCheck>(
Expand Down
2 changes: 2 additions & 0 deletions clang-tools-extra/clang-tidy/performance/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ add_clang_library(clangTidyPerformanceModule
MoveConstructorInitCheck.cpp
NoAutomaticMoveCheck.cpp
NoIntToPtrCheck.cpp
NoexceptDestructorCheck.cpp
NoexceptMoveConstructorCheck.cpp
NoexceptSwapCheck.cpp
PerformanceTidyModule.cpp
TriviallyDestructibleCheck.cpp
TypePromotionInMathFnCheck.cpp
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
//===--- NoexceptDestructorCheck.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 "NoexceptDestructorCheck.h"
#include "../utils/LexerUtils.h"
#include "clang/AST/ASTContext.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");
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;

const SourceLocation NoexceptLoc =
utils::lexer::getLocationForNoexceptSpecifier(FuncDecl, SM);
if (NoexceptLoc.isValid())
Diag << FixItHint::CreateInsertion(NoexceptLoc, " noexcept ");
}

} // namespace clang::tidy::performance
42 changes: 42 additions & 0 deletions clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
//===--- NoexceptDestructorCheck.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_NOEXCEPTDESTRUCTORCHECK_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTDESTRUCTORCHECK_H

#include "../ClangTidyCheck.h"
#include "../utils/ExceptionSpecAnalyzer.h"

namespace clang::tidy::performance {

/// The check flags destructors not marked with `noexcept` or marked
/// with `noexcept(expr)` where `expr` evaluates to `false`
/// (but is not a `false` literal itself).
///
/// For the user-facing documentation see:
/// https://clang.llvm.org/extra/clang-tidy/checks/performance/noexcept-destructor.html
class NoexceptDestructorCheck : public ClangTidyCheck {
public:
NoexceptDestructorCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
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;
};

} // namespace clang::tidy::performance

#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTDESTRUCTORCHECK_H
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//

#include "NoexceptMoveConstructorCheck.h"
#include "../utils/LexerUtils.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"
Expand All @@ -18,7 +19,7 @@ namespace clang::tidy::performance {

void NoexceptMoveConstructorCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
cxxMethodDecl(unless(isImplicit()), unless(isDeleted()),
cxxMethodDecl(unless(isDeleted()),
anyOf(cxxConstructorDecl(isMoveConstructor()),
isMoveAssignmentOperator()))
.bind("decl"),
Expand All @@ -27,19 +28,18 @@ void NoexceptMoveConstructorCheck::registerMatchers(MatchFinder *Finder) {

void NoexceptMoveConstructorCheck::check(
const MatchFinder::MatchResult &Result) {
const auto *Decl = Result.Nodes.getNodeAs<CXXMethodDecl>("decl");
if (!Decl)
return;
const auto *FuncDecl = Result.Nodes.getNodeAs<CXXMethodDecl>("decl");
assert(FuncDecl);

if (SpecAnalyzer.analyze(Decl) !=
if (SpecAnalyzer.analyze(FuncDecl) !=
utils::ExceptionSpecAnalyzer::State::Throwing)
return;

const bool IsConstructor = CXXConstructorDecl::classof(Decl);
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 = Decl->getType()->castAs<FunctionProtoType>();
const auto *ProtoType = FuncDecl->getType()->castAs<FunctionProtoType>();
const Expr *NoexceptExpr = ProtoType->getNoexceptExpr();
if (NoexceptExpr) {
NoexceptExpr = NoexceptExpr->IgnoreImplicit();
Expand All @@ -52,21 +52,18 @@ void NoexceptMoveConstructorCheck::check(
return;
}

auto Diag = diag(Decl->getLocation(),
auto Diag = diag(FuncDecl->getLocation(),
"move %select{assignment operator|constructor}0s should "
"be marked noexcept")
<< IsConstructor;
// Add FixIt hints.

const SourceManager &SM = *Result.SourceManager;
assert(Decl->getNumParams() > 0);
SourceLocation NoexceptLoc =
Decl->getParamDecl(Decl->getNumParams() - 1)->getSourceRange().getEnd();
if (NoexceptLoc.isValid())
NoexceptLoc = Lexer::findLocationAfterToken(
NoexceptLoc, tok::r_paren, SM, Result.Context->getLangOpts(), true);

const SourceLocation NoexceptLoc =
utils::lexer::getLocationForNoexceptSpecifier(FuncDecl, SM);
if (NoexceptLoc.isValid())
Diag << FixItHint::CreateInsertion(NoexceptLoc, " noexcept ");
return;
}

} // namespace clang::tidy::performance
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ namespace clang::tidy::performance {
/// Move constructors of all the types used with STL containers, for example,
/// need to be declared `noexcept`. Otherwise STL will choose copy constructors
/// instead. The same is valid for move assignment operations.
///
/// For the user-facing documentation see:
/// https://clang.llvm.org/extra/clang-tidy/checks/performance/noexcept-move-constructor.html
class NoexceptMoveConstructorCheck : public ClangTidyCheck {
public:
NoexceptMoveConstructorCheck(StringRef Name, ClangTidyContext *Context)
Expand All @@ -30,6 +33,9 @@ class NoexceptMoveConstructorCheck : public ClangTidyCheck {
}
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;
Expand Down
57 changes: 57 additions & 0 deletions clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
//===--- NoexceptSwapCheck.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 "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;

namespace clang::tidy::performance {

void NoexceptSwapCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
functionDecl(unless(isDeleted()), hasName("swap")).bind("decl"), 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;

const SourceLocation NoexceptLoc =
utils::lexer::getLocationForNoexceptSpecifier(FuncDecl, SM);
if (NoexceptLoc.isValid())
Diag << FixItHint::CreateInsertion(NoexceptLoc, " noexcept ");
}

} // namespace clang::tidy::performance
42 changes: 42 additions & 0 deletions clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
//===--- NoexceptSwapCheck.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_NOEXCEPTSWAPCHECK_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTSWAPCHECK_H

#include "../ClangTidyCheck.h"
#include "../utils/ExceptionSpecAnalyzer.h"

namespace clang::tidy::performance {

/// The check flags swap functions not marked with `noexcept` or marked
/// with `noexcept(expr)` where `expr` evaluates to `false`
/// (but is not a `false` literal itself).
///
/// For the user-facing documentation see:
/// https://clang.llvm.org/extra/clang-tidy/checks/performance/noexcept-swap.html
class NoexceptSwapCheck : public ClangTidyCheck {
public:
NoexceptSwapCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
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;
};

} // namespace clang::tidy::performance

#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTSWAPCHECK_H
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
#include "MoveConstructorInitCheck.h"
#include "NoAutomaticMoveCheck.h"
#include "NoIntToPtrCheck.h"
#include "NoexceptDestructorCheck.h"
#include "NoexceptMoveConstructorCheck.h"
#include "NoexceptSwapCheck.h"
#include "TriviallyDestructibleCheck.h"
#include "TypePromotionInMathFnCheck.h"
#include "UnnecessaryCopyInitialization.h"
Expand Down Expand Up @@ -52,8 +54,12 @@ class PerformanceModule : public ClangTidyModule {
CheckFactories.registerCheck<NoAutomaticMoveCheck>(
"performance-no-automatic-move");
CheckFactories.registerCheck<NoIntToPtrCheck>("performance-no-int-to-ptr");
CheckFactories.registerCheck<NoexceptDestructorCheck>(
"performance-noexcept-destructor");
CheckFactories.registerCheck<NoexceptMoveConstructorCheck>(
"performance-noexcept-move-constructor");
CheckFactories.registerCheck<NoexceptSwapCheck>(
"performance-noexcept-swap");
CheckFactories.registerCheck<TriviallyDestructibleCheck>(
"performance-trivially-destructible");
CheckFactories.registerCheck<TypePromotionInMathFnCheck>(
Expand Down
5 changes: 5 additions & 0 deletions clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,11 @@ ExceptionSpecAnalyzer::analyzeFunctionEST(const FunctionDecl *FuncDecl,
if (isUnresolvedExceptionSpec(FuncProto->getExceptionSpecType()))
return State::Unknown;

// A non defaulted destructor without the noexcept specifier is still noexcept
if (isa<CXXDestructorDecl>(FuncDecl) &&
FuncDecl->getExceptionSpecType() == EST_None)
return State::NotThrowing;

switch (FuncProto->canThrow()) {
case CT_Cannot:
return State::NotThrowing;
Expand Down

0 comments on commit 474a2b9

Please sign in to comment.