[clang-tidy] New checker performance-trivially-destructible-check

[AntonBikineev]

Summary

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.

[lebedev.ri]

clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp

23

I believe static functions are preferred to anon namespaces;
most (if not all) of this checking should/could be done in the matcher itself,
i.e. the check() could be called to unconditionally issue diag.

51

https://en.cppreference.com/w/cpp/language/destructor says defaulted destructor is C++11 feature,
so not register if this isn't C++11?

78

This always drops out-of-line defaulting.
Is there/can there be any case where such defaultig should just be moved in-line into the class?

clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.h

19–20

This doesn't read right, should be closer to this maybe

/// A check that finds classes that would be trivial if not for the defaulted destructors declared out-of-line:

clang-tools-extra/docs/ReleaseNotes.rst

147

s/destrictuble/destructible/?

clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst

7

s/destrictuble/destructible/?

clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp

2

Do you also want to check that fix-it applies and result passes sema?

[Eugene.Zelenko]

clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp

30

Please don't use auto unless type is spelled in same statement or iterator.

32

Please don't use auto unless type is spelled in same statement or iterator.

39

Please don't use auto unless type is spelled in same statement or iterator.

62

Please don't use auto unless type is spelled in same statement or iterator.

67

Please don't use auto unless type is spelled in same statement or iterator.

clang-tools-extra/docs/ReleaseNotes.rst

144

Please move into new checks list (in alphabetical order).

[AntonBikineev]

clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp

23

I couldn't find matchers to check triviallity of special functions or something like isFirstDecl(). Perhaps, would make sense to add some.

51

Right. However, I think it will also make sense to extend the check for destructors with empty bodies.

78

I've thought about this. I think it mostly comes down to code-style preference (rule-of-5 vs rule-of-0). But after giving it another thought, I think, the current implementation may lead to more surprises for the user, so probably moving = default to the first declaration would be a better idea :)

clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp

2

Aren't fix-its already checked by CHECK-FIXES?
It would be great to check if the result is compilable. Should I run another clang_tidy instance to check that or is there another way to do so?

[AntonBikineev]

Thanks for the suggestions! Addressed some of them.

[AntonBikineev]

Roman, could you please take another look at it?

[lebedev.ri]

clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp

23

You can trivially add your own matchers, see e.g.
https://github.com/llvm/llvm-project/blob/058b628264d276515a0aab66285816fe6cde2aa2/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp#L15-L39

clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp

2

It FileChecks the // CHECK-FIXES lines, but i don't think it actually applies them.
https://github.com/llvm/llvm-project/blob/885c559369fe3d6323898c17787bd0454065fc34/clang-tools-extra/test/clang-tidy/checkers/cert-uppercase-literal-suffix-integer.cpp#L2-L4

[aaron.ballman]

clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp

28

Drop top-level const qualifiers unless it's a pointer or reference (it's not a style we use elsewhere).

40

We don't usually attach names to TODOs -- are you planning to work on that for this patch?

72

it -> its

[AntonBikineev]

Thanks for the suggestions!

Added isTriviallyDestructible matcher to utils/Matchers.h.

[AntonBikineev]

Does anybody have suggestions on this?

[lebedev.ri]

In D69435#1725795, @AntonBikineev wrote:

Does anybody have suggestions on this?

Can you be more specific what the question is?

clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp

43

Thanks, this looks good.

[AntonBikineev]

Roman, thanks for reviewing and the ideas.

Can you be more specific what the question is?

I meant to ping if anybody has more comments on this patch. Otherwise I would go ahead and submit it.

[sylvestre.ledru]

Did you see some significant perf improvements for Chromium?

[AntonBikineev]

Did you see some significant perf improvements for Chromium?

I don't see major improvements in browser benchmarks like Speedometer, but in the Blink garbage collector, which queues destructors to be executed later, this change proved to reduce the number of them by 5%. That, in turn, reduces GC time on the main thread which is significant for latency.

[sylvestre.ledru]

Thanks!

[AntonBikineev]

BTW, this very contrived benchmark shows 5x: http://quick-bench.com/sczBi_lVndKut9jOj4UofC0HYew

[xbolva00]

In D69435#1728674, @AntonBikineev wrote:

BTW, this very contrived benchmark shows 5x: http://quick-bench.com/sczBi_lVndKut9jOj4UofC0HYew

There is no difference in perf for GCC.

[AntonBikineev]

There is no difference in perf for GCC.

Yes, but that's because gcc still optimizes the call to noinlined function. In the common scenario which this check addresses (destructor defaulted in .cpp file) gcc is not able to optimize the call.

[gribozavr2]

LGTM modulo the isFirstDeclComment. Will approve after we resolve that discussion.

clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp

46

The "isFirstDecl" part probably will always work, however, it is semantically not really the right predicate to check. What we want to check is where the constructor is declared -- in the class or outside the class, correct? If so, then the matcher should be looking at the DeclContext of the constructor decl.

[AntonBikineev]

clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp

46

Thanks for the comment.

What we want to check is where the constructor is declared -- in the class or outside the class, correct?

I was following the Standard's wording, which says that trivial destructor is not user-provided, where user-provided means

user-declared and not explicitly defaulted or deleted on its first declaration.

Technically, I think both ways should work as C++ doesn't allow more than 2 special function declarations.

[gribozavr2]

Alright then. (Although I don't know whether the redeclaration chain always models that wording in the standard, and what that wording means when we don't have a linear ordering of text -- that is, in modules.)

Do you have commit access or do you need me to commit for you?

[AntonBikineev]

In D69435#1729580, @gribozavr2 wrote:

Alright then. (Although I don't know whether the redeclaration chain always models that wording in the standard, and what that wording means when we don't have a linear ordering of text -- that is, in modules.)

Do you have commit access or do you need me to commit for you?

Thanks for taking the look, I'll commit.

clang-tools-extra/clang-tidy/performance/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyPerformanceModule
   FasterStringFindCheck.cpp
   ForRangeCopyCheck.cpp
   ImplicitConversionInLoopCheck.cpp
   InefficientAlgorithmCheck.cpp
   InefficientStringConcatenationCheck.cpp
   InefficientVectorOperationCheck.cpp
   MoveConstArgCheck.cpp
   MoveConstructorInitCheck.cpp
   NoexceptMoveConstructorCheck.cpp
   PerformanceTidyModule.cpp
+  TriviallyDestructibleCheck.cpp
   TypePromotionInMathFnCheck.cpp
   UnnecessaryCopyInitialization.cpp
   UnnecessaryValueParamCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangAnalysis
   clangBasic
   clangLex
   clangTidy
   clangTidyUtils
   )

clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp

 #include "ForRangeCopyCheck.h"
 #include "ImplicitConversionInLoopCheck.h"
 #include "InefficientAlgorithmCheck.h"
 #include "InefficientStringConcatenationCheck.h"
 #include "InefficientVectorOperationCheck.h"
 #include "MoveConstArgCheck.h"
 #include "MoveConstructorInitCheck.h"
 #include "NoexceptMoveConstructorCheck.h"
+#include "TriviallyDestructibleCheck.h"
 #include "TypePromotionInMathFnCheck.h"
 #include "UnnecessaryCopyInitialization.h"
 #include "UnnecessaryValueParamCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace performance {
 
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<InefficientVectorOperationCheck>(
         "performance-inefficient-vector-operation");
     CheckFactories.registerCheck<MoveConstArgCheck>(
         "performance-move-const-arg");
     CheckFactories.registerCheck<MoveConstructorInitCheck>(
         "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>(
         "performance-unnecessary-copy-initialization");
     CheckFactories.registerCheck<UnnecessaryValueParamCheck>(
         "performance-unnecessary-value-param");
   }
 };

clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.h

+//===--- 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/performance/TriviallyDestructibleCheck.cpp

+//===--- 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/utils/Matchers.h

AST_MATCHER(QualType, isExpensiveToCopy) {
   return IsExpensive && *IsExpensive;
 }
 
 AST_MATCHER(RecordDecl, isTriviallyDefaultConstructible) {
   return utils::type_traits::recordIsTriviallyDefaultConstructible(
       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;
   return referenceType(pointee(qualType(isConstQualified())));
 }
 
 AST_MATCHER_P(NamedDecl, matchesAnyListedName, std::vector<std::string>,
               NameList) {

clang-tools-extra/clang-tidy/utils/TypeTraits.h

 
 /// Returns `true` if `Type` is trivially default constructible.
 bool isTriviallyDefaultConstructible(QualType Type, const ASTContext &Context);
 
 /// Returns `true` if `RecordDecl` is trivially default constructible.
 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);
 
 /// Return true if `Type` has a non-trivial move assignment operator.
 bool hasNonTrivialMoveAssignment(QualType Type);
 
 } // type_traits
 } // namespace utils
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_TYPETRAITS_H

clang-tools-extra/clang-tidy/utils/TypeTraits.cpp

 }
 
 bool recordIsTriviallyDefaultConstructible(const RecordDecl &RecordDecl,
                                            const ASTContext &Context) {
   const auto *ClassDecl = dyn_cast<CXXRecordDecl>(&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;
   // A class with a user-provided default constructor is not trivially
   // constructible.
   if (ClassDecl->hasUserProvidedDefaultConstructor())
     return false;
   // A polymorphic class is not trivially constructible
bool isTriviallyDefaultConstructible(QualType Type, const ASTContext &Context) {
   if (const auto *RT = CanonicalType->getAs<RecordType>()) {
     return recordIsTriviallyDefaultConstructible(*RT->getDecl(), Context);
   }
 
   // No other types can match.
   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() &&
          Record->hasNonTrivialMoveConstructor();
 }
 
 bool hasNonTrivialMoveAssignment(QualType Type) {
   auto *Record = Type->getAsCXXRecordDecl();
   return Record && Record->hasDefinition() &&
          Record->hasNonTrivialMoveAssignment();
 }
 
 } // namespace type_traits
 } // namespace utils
 } // namespace tidy
 } // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`llvm-prefer-register-over-unsigned
   them to use ``Register``
 
 - New :doc:`objc-missing-hash
   <clang-tidy/checks/objc-missing-hash>` check.
 
   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.
 
   Now also checks if any calls to ``pthread_*`` functions expect negative return
   values.
 
 - The 'objc-avoid-spinlock' check was renamed to :doc:`darwin-avoid-spinlock
   <clang-tidy/checks/darwin-avoid-spinlock>`
 
 - New :doc:`readability-redundant-access-specifiers
   <clang-tidy/checks/readability-redundant-access-specifiers>` check.
 
   Finds classes, structs, and unions that contain redundant member
   access specifiers.
 
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 
 Improvements to clang-include-fixer

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. toctree::
    performance-for-range-copy
    performance-implicit-conversion-in-loop
    performance-inefficient-algorithm
    performance-inefficient-string-concatenation
    performance-inefficient-vector-operation
    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
    portability-simd-intrinsics
    readability-avoid-const-params-in-decls
    readability-braces-around-statements
    readability-const-return-type
    readability-container-size-empty

clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst

+.. 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

+// 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;