diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt @@ -11,6 +11,7 @@ MoveConstructorInitCheck.cpp NoexceptMoveConstructorCheck.cpp PerformanceTidyModule.cpp + TriviallyDestructibleCheck.cpp TypePromotionInMathFnCheck.cpp UnnecessaryCopyInitialization.cpp UnnecessaryValueParamCheck.cpp diff --git a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp --- a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp @@ -18,6 +18,7 @@ #include "MoveConstArgCheck.h" #include "MoveConstructorInitCheck.h" #include "NoexceptMoveConstructorCheck.h" +#include "TriviallyDestructibleCheck.h" #include "TypePromotionInMathFnCheck.h" #include "UnnecessaryCopyInitialization.h" #include "UnnecessaryValueParamCheck.h" @@ -47,6 +48,8 @@ "performance-move-constructor-init"); CheckFactories.registerCheck( "performance-noexcept-move-constructor"); + CheckFactories.registerCheck( + "performance-trivially-destructible"); CheckFactories.registerCheck( "performance-type-promotion-in-math-fn"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.h b/clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.h @@ -0,0 +1,40 @@ +//===--- TriviallyDestructibleCheck.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_TRIVIALLYDESTRUCTIBLECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_TRIVIALLYDESTRUCTIBLECHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace performance { + +/// A check that finds classes that would be trivial if not for the defaulted +/// destructors declared out-of-line: +/// struct A: TrivialClass { +/// ~A(); +/// TrivialClass trivial_fields; +/// }; +/// A::~A() = default; +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/performance-trivially-destructible.html +class TriviallyDestructibleCheck : public ClangTidyCheck { +public: + TriviallyDestructibleCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace performance +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_TRIVIALLYDESTRUCTIBLECHECK_H diff --git a/clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp b/clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp @@ -0,0 +1,82 @@ +//===--- TriviallyDestructibleCheck.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 "TriviallyDestructibleCheck.h" +#include "../utils/LexerUtils.h" +#include "../utils/Matchers.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; +using namespace clang::ast_matchers::internal; +using namespace clang::tidy::matchers; + +namespace clang { +namespace tidy { +namespace performance { + +namespace { + +AST_MATCHER(Decl, isFirstDecl) { return Node.isFirstDecl(); } + +AST_MATCHER_P(CXXRecordDecl, hasBase, Matcher, InnerMatcher) { + for (const CXXBaseSpecifier &BaseSpec : Node.bases()) { + QualType BaseType = BaseSpec.getType(); + if (InnerMatcher.matches(BaseType, Finder, Builder)) + return true; + } + return false; +} + +} // namespace + +void TriviallyDestructibleCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus11) + return; + + Finder->addMatcher( + cxxDestructorDecl( + isDefaulted(), + unless(anyOf(isFirstDecl(), isVirtual(), + ofClass(cxxRecordDecl( + anyOf(hasBase(unless(isTriviallyDestructible())), + has(fieldDecl(unless( + hasType(isTriviallyDestructible())))))))))) + .bind("decl"), + this); +} + +void TriviallyDestructibleCheck::check(const MatchFinder::MatchResult &Result) { + const auto *MatchedDecl = Result.Nodes.getNodeAs("decl"); + + // Get locations of both first and out-of-line declarations. + SourceManager &SM = *Result.SourceManager; + const auto *FirstDecl = cast(MatchedDecl->getFirstDecl()); + const SourceLocation FirstDeclEnd = utils::lexer::findNextTerminator( + FirstDecl->getEndLoc(), SM, getLangOpts()); + const CharSourceRange SecondDeclRange = CharSourceRange::getTokenRange( + MatchedDecl->getBeginLoc(), + utils::lexer::findNextTerminator(MatchedDecl->getEndLoc(), SM, + getLangOpts())); + if (FirstDeclEnd.isInvalid() || SecondDeclRange.isInvalid()) + return; + + // Report diagnostic. + diag(FirstDecl->getLocation(), + "class %0 can be made trivially destructible by defaulting the " + "destructor on its first declaration") + << FirstDecl->getParent() + << FixItHint::CreateInsertion(FirstDeclEnd, " = default") + << FixItHint::CreateRemoval(SecondDeclRange); + diag(MatchedDecl->getLocation(), "destructor definition is here", + DiagnosticIDs::Note); +} + +} // namespace performance +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/utils/Matchers.h b/clang-tools-extra/clang-tidy/utils/Matchers.h --- a/clang-tools-extra/clang-tidy/utils/Matchers.h +++ b/clang-tools-extra/clang-tidy/utils/Matchers.h @@ -41,6 +41,10 @@ Node, Finder->getASTContext()); } +AST_MATCHER(QualType, isTriviallyDestructible) { + return utils::type_traits::isTriviallyDestructible(Node); +} + // Returns QualType matcher for references to const. AST_MATCHER_FUNCTION(ast_matchers::TypeMatcher, isReferenceToConst) { using namespace ast_matchers; diff --git a/clang-tools-extra/clang-tidy/utils/TypeTraits.h b/clang-tools-extra/clang-tidy/utils/TypeTraits.h --- a/clang-tools-extra/clang-tidy/utils/TypeTraits.h +++ b/clang-tools-extra/clang-tidy/utils/TypeTraits.h @@ -28,6 +28,9 @@ bool recordIsTriviallyDefaultConstructible(const RecordDecl &RecordDecl, const ASTContext &Context); +/// Returns `true` if `Type` is trivially destructible. +bool isTriviallyDestructible(QualType Type); + /// Returns true if `Type` has a non-trivial move constructor. bool hasNonTrivialMoveConstructor(QualType Type); diff --git a/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp b/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp --- a/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp +++ b/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp @@ -54,7 +54,7 @@ // Non-C++ records are always trivially constructible. if (!ClassDecl) return true; - // It is impossible to detemine whether an ill-formed decl is trivially + // It is impossible to determine whether an ill-formed decl is trivially // constructible. if (RecordDecl.isInvalidDecl()) return false; @@ -135,6 +135,20 @@ return false; } +// Based on QualType::isDestructedType. +bool isTriviallyDestructible(QualType Type) { + if (Type.isNull()) + return false; + + if (Type->isIncompleteType()) + return false; + + if (Type.getCanonicalType()->isDependentType()) + return false; + + return Type.isDestructedType() == QualType::DK_none; +} + bool hasNonTrivialMoveConstructor(QualType Type) { auto *Record = Type->getAsCXXRecordDecl(); return Record && Record->hasDefinition() && diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -126,6 +126,12 @@ Finds Objective-C implementations that implement ``-isEqual:`` without also appropriately implementing ``-hash``. +- New :doc:`performance-trivially-destructible + ` check. + + Finds types that could be made trivially-destructible by removing out-of-line + defaulted destructor declarations. + - Improved :doc:`bugprone-posix-return ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -342,6 +342,7 @@ performance-move-const-arg performance-move-constructor-init performance-noexcept-move-constructor + performance-trivially-destructible performance-type-promotion-in-math-fn performance-unnecessary-copy-initialization performance-unnecessary-value-param diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst b/clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst @@ -0,0 +1,15 @@ +.. title:: clang-tidy - performance-trivially-destructible + +performance-trivially-destructible +================================== + +Finds types that could be made trivially-destructible by removing out-of-line +defaulted destructor declarations. + +.. code-block:: c++ + + struct A: TrivialType { + ~A(); // Makes A non-trivially-destructible. + TrivialType trivial_fields; + }; + A::~A() = default; diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp @@ -0,0 +1,84 @@ +// RUN: %check_clang_tidy %s performance-trivially-destructible %t +// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp +// RUN: clang-tidy %t.cpp -checks='-*,performance-trivially-destructible' -fix +// RUN: clang-tidy %t.cpp -checks='-*,performance-trivially-destructible' -warnings-as-errors='-*,performance-trivially-destructible' + +struct TriviallyDestructible1 { + int a; +}; + +struct TriviallyDestructible2 : TriviallyDestructible1 { + ~TriviallyDestructible2() = default; + TriviallyDestructible1 b; +}; + +struct NotTriviallyDestructible1 : TriviallyDestructible2 { + ~NotTriviallyDestructible1(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'NotTriviallyDestructible1' can be made trivially destructible by defaulting the destructor on its first declaration [performance-trivially-destructible] + // CHECK-FIXES: ~NotTriviallyDestructible1() = default; + TriviallyDestructible2 b; +}; + +NotTriviallyDestructible1::~NotTriviallyDestructible1() = default; // to-be-removed +// CHECK-MESSAGES: :[[@LINE-1]]:28: note: destructor definition is here +// CHECK-FIXES: {{^}}// to-be-removed + +// Don't emit for class template with type-dependent fields. +template +struct MaybeTriviallyDestructible1 { + ~MaybeTriviallyDestructible1() noexcept; + T t; +}; + +template +MaybeTriviallyDestructible1::~MaybeTriviallyDestructible1() noexcept = default; + +// Don't emit for specializations. +template struct MaybeTriviallyDestructible1; + +// Don't emit for class template with type-dependent bases. +template +struct MaybeTriviallyDestructible2 : T { + ~MaybeTriviallyDestructible2() noexcept; +}; + +template +MaybeTriviallyDestructible2::~MaybeTriviallyDestructible2() noexcept = default; + +// Emit for templates without dependent bases and fields. +template +struct MaybeTriviallyDestructible1 { + ~MaybeTriviallyDestructible1() noexcept; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'MaybeTriviallyDestructible1' can be made trivially destructible by defaulting the destructor on its first declaration [performance-trivially-destructible] + // CHECK-FIXES: ~MaybeTriviallyDestructible1() noexcept = default; + TriviallyDestructible1 t; +}; + +template +MaybeTriviallyDestructible1::~MaybeTriviallyDestructible1() noexcept = default; // to-be-removed +// CHECK-MESSAGES: :[[@LINE-1]]:35: note: destructor definition is here +// CHECK-FIXES: {{^}}// to-be-removed + +// Emit for explicit specializations. +template <> +struct MaybeTriviallyDestructible1: TriviallyDestructible1 { + ~MaybeTriviallyDestructible1() noexcept; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'MaybeTriviallyDestructible1' can be made trivially destructible by defaulting the destructor on its first declaration [performance-trivially-destructible] + // CHECK-FIXES: ~MaybeTriviallyDestructible1() noexcept = default; +}; + +MaybeTriviallyDestructible1::~MaybeTriviallyDestructible1() noexcept = default; // to-be-removed +// CHECK-MESSAGES: :[[@LINE-1]]:38: note: destructor definition is here +// CHECK-FIXES: {{^}}// to-be-removed + +struct NotTriviallyDestructible2 { + virtual ~NotTriviallyDestructible2(); +}; + +NotTriviallyDestructible2::~NotTriviallyDestructible2() = default; + +struct NotTriviallyDestructible3: NotTriviallyDestructible2 { + ~NotTriviallyDestructible3(); +}; + +NotTriviallyDestructible3::~NotTriviallyDestructible3() = default;