[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

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

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 @@ public:
         "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>(

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

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

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;

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() &&

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);
 

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.
 

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

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;

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;