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 @@ -11,6 +11,7 @@ MoveConstructorInitCheck.cpp NoexceptMoveConstructorCheck.cpp PerformanceTidyModule.cpp + TriviallyDestructibleCheck.cpp TypePromotionInMathFnCheck.cpp UnnecessaryCopyInitialization.cpp UnnecessaryValueParamCheck.cpp Index: clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp =================================================================== --- clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp +++ 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( Index: clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.h =================================================================== --- /dev/null +++ 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 Index: clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp @@ -0,0 +1,81 @@ +//===--- 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 "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace performance { + +static bool +CheckPotentiallyTriviallyDestructible(const CXXDestructorDecl *Dtor) { + if (Dtor->isFirstDecl()) + return false; + // Check direct base classes. + const CXXRecordDecl *RecordDecl = Dtor->getParent(); + for (FieldDecl *Field : RecordDecl->fields()) { + const QualType FieldType = Field->getType(); + if (FieldType->isDependentType() || + FieldType.isDestructedType() != clang::QualType::DK_none) + return false; + } + // Check fields. + for (const CXXBaseSpecifier &BaseSpec : RecordDecl->bases()) { + const QualType BaseType = BaseSpec.getType(); + if (BaseType->isDependentType() || + BaseType.isDestructedType() != clang::QualType::DK_none) + return false; + } + // TODO(bikineev): Check for empty compound statement? + return true; +} + +void TriviallyDestructibleCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus11) + return; + + Finder->addMatcher( + cxxDestructorDecl(isDefaulted(), unless(isVirtual())).bind("decl"), this); +} + +void TriviallyDestructibleCheck::check(const MatchFinder::MatchResult &Result) { + const auto *MatchedDecl = Result.Nodes.getNodeAs("decl"); + if (!CheckPotentiallyTriviallyDestructible(MatchedDecl)) + return; + + // 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 it 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 Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -121,6 +121,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. Index: clang-tools-extra/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -343,6 +343,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 Index: clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst =================================================================== --- /dev/null +++ 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; Index: clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp @@ -0,0 +1,80 @@ +// RUN: %check_clang_tidy %s performance-trivially-destructible %t + +struct TriviallyDestructible1 { + int a; +}; + +struct TriviallyDestructible2 : TriviallyDestructible1 { + TriviallyDestructible1 b; +}; + +struct NotTriviallyDestructible1 : TriviallyDestructible2 { + ~NotTriviallyDestructible1(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'NotTriviallyDestructible1' can be made trivially destructible by defaulting the destructor on it 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 it 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 it 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;