[clang-tools-extra] d36a033 - [clang-tidy] New checker performance-trivially-destructible-check
Anton Bikineev via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 1 08:17:10 PDT 2019
Author: Anton Bikineev
Date: 2019-11-01T16:16:49+01:00
New Revision: d36a0333102698a1398971d0717465322b1c5c2c
URL: https://github.com/llvm/llvm-project/commit/d36a0333102698a1398971d0717465322b1c5c2c
DIFF: https://github.com/llvm/llvm-project/commit/d36a0333102698a1398971d0717465322b1c5c2c.diff
LOG: [clang-tidy] New checker performance-trivially-destructible-check
Checks for types which can be made trivially-destructible by removing
out-of-line defaulted destructor declarations.
The check is motivated by the work on C++ garbage collector in Blink
(rendering engine for Chrome), which strives to minimize destructors and
improve runtime of sweeping phase.
In the entire chromium codebase the check hits over 2000 times.
Differential Revision: https://reviews.llvm.org/D69435
Added:
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.h
clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst
clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp
Modified:
clang-tools-extra/clang-tidy/performance/CMakeLists.txt
clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
clang-tools-extra/clang-tidy/utils/Matchers.h
clang-tools-extra/clang-tidy/utils/TypeTraits.cpp
clang-tools-extra/clang-tidy/utils/TypeTraits.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
index b6302a5ff815..cde2e246bf9e 100644
--- a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
@@ -11,6 +11,7 @@ add_clang_library(clangTidyPerformanceModule
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
index f4b620a14f85..269d09b98a68 100644
--- 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 @@ class PerformanceModule : public ClangTidyModule {
"performance-move-constructor-init");
CheckFactories.registerCheck<NoexceptMoveConstructorCheck>(
"performance-noexcept-move-constructor");
+ CheckFactories.registerCheck<TriviallyDestructibleCheck>(
+ "performance-trivially-destructible");
CheckFactories.registerCheck<TypePromotionInMathFnCheck>(
"performance-type-promotion-in-math-fn");
CheckFactories.registerCheck<UnnecessaryCopyInitialization>(
diff --git a/clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp b/clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp
new file mode 100644
index 000000000000..5ed705b0cd79
--- /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<QualType>, 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<CXXDestructorDecl>("decl");
+
+ // Get locations of both first and out-of-line declarations.
+ SourceManager &SM = *Result.SourceManager;
+ const auto *FirstDecl = cast<CXXMethodDecl>(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/performance/TriviallyDestructibleCheck.h b/clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.h
new file mode 100644
index 000000000000..ee3fb40c2a91
--- /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/utils/Matchers.h b/clang-tools-extra/clang-tidy/utils/Matchers.h
index dbb72c9d53be..e19c82731f89 100644
--- a/clang-tools-extra/clang-tidy/utils/Matchers.h
+++ b/clang-tools-extra/clang-tidy/utils/Matchers.h
@@ -41,6 +41,10 @@ AST_MATCHER(RecordDecl, isTriviallyDefaultConstructible) {
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.cpp b/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp
index 9cc422a5a87a..073558776655 100644
--- a/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp
+++ b/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp
@@ -54,7 +54,7 @@ bool recordIsTriviallyDefaultConstructible(const RecordDecl &RecordDecl,
// 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 @@ bool isTriviallyDefaultConstructible(QualType Type, const ASTContext &Context) {
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/clang-tidy/utils/TypeTraits.h b/clang-tools-extra/clang-tidy/utils/TypeTraits.h
index 6102c288173e..f4d3455e9e13 100644
--- a/clang-tools-extra/clang-tidy/utils/TypeTraits.h
+++ b/clang-tools-extra/clang-tidy/utils/TypeTraits.h
@@ -28,6 +28,9 @@ bool isTriviallyDefaultConstructible(QualType Type, const ASTContext &Context);
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/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index c706ae13c785..4a7fef5bb03e 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -126,6 +126,12 @@ Improvements to clang-tidy
Finds Objective-C implementations that implement ``-isEqual:`` without also
appropriately implementing ``-hash``.
+- New :doc:`performance-trivially-destructible
+ <clang-tidy/checks/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
<clang-tidy/checks/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
index bbdcdfa3f86a..2801168d6320 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -342,6 +342,7 @@ Clang-Tidy Checks
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
index 000000000000..00e4e470739c
--- /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
index 000000000000..927a0905ee42
--- /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 <class T>
+struct MaybeTriviallyDestructible1 {
+ ~MaybeTriviallyDestructible1() noexcept;
+ T t;
+};
+
+template <class T>
+MaybeTriviallyDestructible1<T>::~MaybeTriviallyDestructible1() noexcept = default;
+
+// Don't emit for specializations.
+template struct MaybeTriviallyDestructible1<int>;
+
+// Don't emit for class template with type-dependent bases.
+template <class T>
+struct MaybeTriviallyDestructible2 : T {
+ ~MaybeTriviallyDestructible2() noexcept;
+};
+
+template <class T>
+MaybeTriviallyDestructible2<T>::~MaybeTriviallyDestructible2() noexcept = default;
+
+// Emit for templates without dependent bases and fields.
+template <class T>
+struct MaybeTriviallyDestructible1<T *> {
+ ~MaybeTriviallyDestructible1() noexcept;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'MaybeTriviallyDestructible1<T *>' can be made trivially destructible by defaulting the destructor on its first declaration [performance-trivially-destructible]
+ // CHECK-FIXES: ~MaybeTriviallyDestructible1() noexcept = default;
+ TriviallyDestructible1 t;
+};
+
+template <class T>
+MaybeTriviallyDestructible1<T *>::~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<double>: TriviallyDestructible1 {
+ ~MaybeTriviallyDestructible1() noexcept;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'MaybeTriviallyDestructible1<double>' can be made trivially destructible by defaulting the destructor on its first declaration [performance-trivially-destructible]
+ // CHECK-FIXES: ~MaybeTriviallyDestructible1() noexcept = default;
+};
+
+MaybeTriviallyDestructible1<double>::~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;
More information about the cfe-commits
mailing list