AntonBikineev created this revision.
AntonBikineev added a reviewer: klimek.
AntonBikineev added projects: clang, clang-tools-extra.
Herald added subscribers: mgehre, xazax.hun, mgorny.
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 in Chrome), which strives to minimize destructor usage
and improve runtime of sweeping phase.
In the entire chromium codebase the check hits over ~2500 times.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D69435
Files:
clang-tools-extra/clang-tidy/performance/CMakeLists.txt
clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst
clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp
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,68 @@
+// RUN: %check_clang_tidy %s performance-trivially-destructible %t
+
+struct TriviallyDestructible1 {
+ int a;
+};
+
+struct TriviallyDestructible2 : TriviallyDestructible1 {
+ TriviallyDestructible1 b;
+};
+
+struct NonTriviallyDestructible1 : TriviallyDestructible2 {
+ ~NonTriviallyDestructible1(); // to-be-removed
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: destructor '~NonTriviallyDestructible1' can be removed to make the class trivially destructible [performance-trivially-destructible]
+ // CHECK-FIXES: {{^}} // to-be-removed
+ TriviallyDestructible2 b;
+};
+
+NonTriviallyDestructible1::~NonTriviallyDestructible1() = 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; // to-be-removed
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: destructor '~MaybeTriviallyDestructible1<type-parameter-0-0 *>' can be removed to make the class trivially destructible [performance-trivially-destructible]
+ // CHECK-FIXES: {{^}} // to-be-removed
+ 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; // to-be-removed
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: destructor '~MaybeTriviallyDestructible1' can be removed to make the class trivially destructible [performance-trivially-destructible]
+ // CHECK-FIXES: {{^}} // to-be-removed
+};
+
+MaybeTriviallyDestructible1<double>::~MaybeTriviallyDestructible1() noexcept = default; // to-be-removed
+// CHECK-MESSAGES: :[[@LINE-1]]:38: note: destructor definition is here
+// CHECK-FIXES: {{^}}// to-be-removed
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-destrictuble 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/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/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -130,6 +130,12 @@
- The 'objc-avoid-spinlock' check was renamed to :doc:`darwin-avoid-spinlock
<clang-tidy/checks/darwin-avoid-spinlock>`
+- New :doc:`performance-trivially-destructible
+ <clang-tidy/checks/performance-trivially-destructible>` check.
+
+ Finds types that could be made trivially-destrictuble by removing out-of-line
+ defaulted destructor declarations.
+
Improvements to include-fixer
-----------------------------
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 out-of-line declared defaulted destructors, which would
+/// otherwise be trivial:
+/// 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,84 @@
+//===--- 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 {
+
+namespace {
+
+bool CheckPotentiallyTriviallyDestructible(const CXXDestructorDecl *Dtor) {
+ if (Dtor->isFirstDecl() || !Dtor->isExplicitlyDefaulted())
+ return false;
+ // Check that the destructor is not virtual.
+ if (Dtor->isVirtual())
+ return false;
+ // Check direct base classes.
+ const auto *RecordDecl = Dtor->getParent();
+ for (auto *Field : RecordDecl->fields()) {
+ const auto FieldType = Field->getType();
+ if (FieldType->isDependentType() ||
+ FieldType.isDestructedType() != clang::QualType::DK_none)
+ return false;
+ }
+ // Check fields.
+ for (const auto &BaseSpec : RecordDecl->bases()) {
+ const auto BaseType = BaseSpec.getType();
+ if (BaseType->isDependentType() ||
+ BaseType.isDestructedType() != clang::QualType::DK_none)
+ return false;
+ }
+ // TODO(bikineev): Check for empty compound statement?
+ return true;
+}
+
+} // namespace
+
+void TriviallyDestructibleCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(cxxDestructorDecl().bind("decl"), this);
+}
+
+void TriviallyDestructibleCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *MatchedDecl = Result.Nodes.getNodeAs<CXXDestructorDecl>("decl");
+ if (!CheckPotentiallyTriviallyDestructible(MatchedDecl))
+ return;
+
+ // Get locations of both first and out-of-line declarations.
+ SourceManager &SM = *Result.SourceManager;
+ const auto *FirstDecl = cast<CXXMethodDecl>(MatchedDecl->getFirstDecl());
+ const auto FirstDeclRange = clang::CharSourceRange::getCharRange(
+ FirstDecl->getBeginLoc(),
+ clang::Lexer::findLocationAfterToken(
+ FirstDecl->getEndLoc(), clang::tok::semi, SM, getLangOpts(),
+ /*SkipTrailingWhitespaceAndNewLine=*/true));
+ const auto SecondDeclRange = clang::CharSourceRange::getTokenRange(
+ MatchedDecl->getBeginLoc(),
+ utils::lexer::findNextTerminator(MatchedDecl->getEndLoc(), SM,
+ getLangOpts()));
+ if (FirstDeclRange.isInvalid() || SecondDeclRange.isInvalid())
+ return;
+
+ // Report diagnostic.
+ diag(FirstDecl->getLocation(),
+ "destructor %0 can be removed to make the class trivially destructible")
+ << FirstDecl << FixItHint::CreateRemoval(FirstDeclRange)
+ << FixItHint::CreateRemoval(SecondDeclRange);
+ diag(MatchedDecl->getLocation(), "destructor definition is here",
+ DiagnosticIDs::Note);
+}
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang
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<NoexceptMoveConstructorCheck>(
"performance-noexcept-move-constructor");
+ CheckFactories.registerCheck<TriviallyDestructibleCheck>(
+ "performance-trivially-destructible");
CheckFactories.registerCheck<TypePromotionInMathFnCheck>(
"performance-type-promotion-in-math-fn");
CheckFactories.registerCheck<UnnecessaryCopyInitialization>(
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
