clang-tidy check: cppcoreguidelines-rule-of-five-and-zero

[jbcoe]

Summary

This clang tidy check checks for classes that violate the rule of five and zero as specified in CppCoreGuidelines: https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-default-operation-define-or-delete-them-all.

If a class defines or deletes a default operation then it should define or delete them all.

[aaron.ballman]

clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp

51 ↗

(On Diff #45449)

We probably do not want to generate this fixit unless we are compiling for at least C++11. We should have a test for C++98.

[aaron.ballman]

clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp

37 ↗

(On Diff #45449)

You should run clang-format over the patch to address 80-col limit issues like this.

49 ↗

(On Diff #45449)

I think we usually prefer operator= to operator =, but am not certain.

clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.h

24 ↗

(On Diff #45449)

Would it make sense to have an explicitly defaulted copy constructor generate a fixit for an explicitly defaulted copy assignment operator instead of a deleted one? It seems like the user is being explicit about wanting default copy operations more often in that case, and the defaulted copy constructor would not be actively harmful to that.

docs/clang-tidy/checks/misc-user-defined-copy-without-assignment.rst

24 ↗

(On Diff #45449)

s/assigneemnt/assignment

[alexfh]

clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp

32 ↗

(On Diff #45449)

This check should be done in the matcher.

49 ↗

(On Diff #45449)

+1 for operator=

Also, I'd use (llvm::Twine("\n") + ClassName + ...).str() instead of the stream.

51 ↗

(On Diff #45449)

Just insert this at the start of registerMatchers:

if (!getLangOpts().CPlusPlus11)
  return;

docs/clang-tidy/checks/misc-user-defined-copy-without-assignment.rst

6 ↗

(On Diff #45449)

Is this problem only relevant to MSVC 2015? Is MSVC's behavior standard-compliant in this case?

[aaron.ballman]

clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp

51 ↗

(On Diff #45449)

I think the check is still useful in C++98 mode assuming older versions of MSVC (before they started supporting C++11) have the same behavior. The fixit isn't useful, but warning the user about the problem is.

docs/clang-tidy/checks/misc-user-defined-copy-without-assignment.rst

6 ↗

(On Diff #45449)

MSVC's behavior is standards compliant, but is deprecated behavior. [class.copy]p18:

"If the class definition does not explicitly declare a copy assignment operator, one is declared implicitly. If
the class definition declares a move constructor or move assignment operator, the implicitly declared copy
assignment operator is defined as deleted; otherwise, it is defined as defaulted (8.4). The latter case is
deprecated if the class has a user-declared copy constructor or a user-declared destructor."

Now that I think about it more, I thought @dblaikie or @rsmith made a check for this once under -Wdeprecated. It looks like Richard did in r183884.

[alexfh]

docs/clang-tidy/checks/misc-user-defined-copy-without-assignment.rst

6 ↗

(On Diff #45449)

Ah, now I start understanding ;)

The check is actually (incompletely) verifying the rule of 2, and it's not specific to MSVC in any way (and the documentation shouldn't mention MSVC then). It makes sense to:

1. make the check symmetric (a user-defined assignment and an implicit copy-construction;
2. extend the check to verify move operations as well (the rule of 4);
3. and maybe extend it to verify the rule of 3 and the rule of 5 (though detecting non-RAII resource owning may be really hard if even possible).

23 ↗

(On Diff #45449)

The fix will break code that uses the implicit assignment operator. I'm not sure it's a nice thing to do in a check enabled by default.

[jbcoe]

Made requested changes.

[jbcoe]

clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.h

25 ↗

(On Diff #45554)

The standard says that compiler generation of the assignment operator is deprecated if there is a user-defined copy constructor 12.8.18 .

'= default' still counts as user-defined to the best of my understanding.

MSVC, clang and gcc all generate assignment operators in the face of deleted copy constructors. None of them seem to follow the deprecation rule.

I agree entirely with your point in principle, but it seems to be at odds with the standard. I think requiring the user to explicitly default the assignment operator by generating a deleted assignment operator is the right thing to do.

docs/clang-tidy/checks/misc-user-defined-copy-without-assignment.rst

24 ↗

(On Diff #45554)

How do I go about disabling a check by default?

[jbcoe]

Added symmetric check for user-defined assignment but no copy constructor.

Check now adds '=default' to the missing special function if the user-specified one was specified as '=default'.

Check needs renaming, I'll update it along with the docs once the behaviour/impl are good.

[jbcoe]

clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.h

25 ↗

(On Diff #45554)

On reflection this is pedantic. As you say if a user defaults one, they will want to default the other.

[jbcoe]

i need the matcher from http://reviews.llvm.org/D16470 to add a corresponding pair of checks and fixes for move-constructors and move-assignment.

[jbcoe]

Added handling for move-constructor/move-assignment pairs.

[jbcoe]

clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp

52 ↗

(On Diff #45729)

I've moved the C++11 check to just before creating the fixit.

[aaron.ballman]

clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp

22 ↗

(On Diff #45729)

Before registering the matchers, can you early return if not in CPlusPlus mode? (No need to register these matchers for C code.)

63 ↗

(On Diff #45729)

This seems to have a lot of duplicate code that could be consolidated.

clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.h

21 ↗

(On Diff #45729)

Should update these comments to reflect the check's behavior better (and remove mention of MSVC since this isn't specific to that compiler).

28 ↗

(On Diff #45729)

This no longer is about just copy, so you may want to rename the class (and files and check) to something different.

docs/clang-tidy/checks/misc-user-defined-copy-without-assignment.rst

8 ↗

(On Diff #45729)

Best to use the tags ([class.copy] paragraph 18) when referring to the standard, since the numbers can change.

[jbcoe]

Rename check and restructure code to avoid repetition.

[aaron.ballman]

clang-tidy/misc/AssignmentAndConstructionCheck.cpp

91 ↗

(On Diff #45987)

Should elide the single quotes in these diagnostics and pass in the NamedDecl to the diagnostic instead of a string -- the diagnostic formatter handles quoting properly from NamedDecl objects.

Also, the diagnostics should remove the hyphens -- it's a copy constructor, not a copy-constructor (same for assignment , etc).

108 ↗

(On Diff #45987)

This line is not indented properly.

133 ↗

(On Diff #45987)

You can pass in &MatchedDecl instead of a StringRef to get the proper quoting in the diagnostic.

142 ↗

(On Diff #45987)

The extra statement after the call to buildFixIt() looks amiss. ;-)

152 ↗

(On Diff #45987)

Elide braces when the compound body is only one statement (here and elsewhere).

160 ↗

(On Diff #45987)

Extra newline here.

clang-tidy/misc/AssignmentAndConstructionCheck.h

31 ↗

(On Diff #45987)

Since this is an implementation detail, perhaps it can be moved into the source file instead of made a public member of the check?

39 ↗

(On Diff #45987)

s/diagnostic/diagnose (to make it a verb)?

clang-tidy/misc/MiscTidyModule.cpp

93

I don't have a better idea for a name, but this doesn't seem to tell the user much about what the check will do, either.

Would it make sense to expand this check into a rule of (2)/3/(4)/5 check (http://en.cppreference.com/w/cpp/language/rule_of_three) with config options as to which rule to enforce, so that it optionally includes the destructor? I'm thinking of config options like: RuleOf=3|5, IncludeDestructors=true|false with the defaults being 3 and true, respectively? The check could be named misc-cpp-special-member-rule (or something more clear than that). As it stands, the check currently handles the rule of 2 and 4, but I think a lot of people may want the rule of 3 or 5 instead and it would be good to cover them all under the same check name.

Note, this could be done in a follow-up patch, I am mostly interested in getting the name correct for the check.

docs/clang-tidy/checks/misc-assignment-and-construction.rst

8 ↗

(On Diff #45987)

No need to mention draft standard; C++14 is an IS. Also, the citation should be: [class.copy] paragraph 18.

18 ↗

(On Diff #45987)

May want to expound on what you mean by the fix being defensive.

test/clang-tidy/misc-assignment-and-construction.cpp

161 ↗

(On Diff #45987)

Can we also have a test that shows a deleted operation generates a deleted companion? e.g.,

struct S {
  S(const S&) = delete;
  // check that it adds S& operator=(const S&) = delete;
};

[jbcoe]

I've responded to review comments (thanks for those) and renamed the check to 'rule-of-five'.

I think it needs moving to cppcoreguidelines and needs destructor handling adding to it. As suggested, I'll address that in a later patch if everything else looks ok.

[jbcoe]

I think I'll move this check to cppcoreguidelines and call it rule-of-five.

https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-default-operation-define-or-delete-them-all

I'll add destructor handling as a later patch.

[alexfh]

In D16376#342901, @jbcoe wrote:

I think I'll move this check to cppcoreguidelines and call it rule-of-five.

Yes, please move it to cppcoreguidelines.

https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-default-operation-define-or-delete-them-all

Please use anchors defined in the .md file. They are somewhat stable unlike the automatic anchors generated by github (which have changed at least once since first C++ Core Guidelines checks were committed). For the rule of five this would be https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rc-five

I'll add destructor handling as a later patch.

SG

[alexfh]

clang-tidy/misc/RuleOfFiveCheck.cpp

30 ↗

(On Diff #46775)

Why is hasDescendant needed? Doesn't just has(...) work? I think, methods are direct children of the CXXRecordDecl they are declared in.

Same applies to other matchers.

80 ↗

(On Diff #46775)

The variable doesn't make the code shorter or easier to read. Please remove it.

110 ↗

(On Diff #46775)

How about removing the deleteOrDefault function and inserting this before the switch:

StringRef deleteOrDefault = MethodDecl.isDefaulted() ? " = default;" : " = delete;";

?

117 ↗

(On Diff #46775)

We usually rely on clang-format to clean up formatting after applying fixes, but in some cases clang-format looks into possible preferences already present in the code. Placement of * or & on pointers and references is one of such things. I think, clang-tidy should keep neutrality in this regard, e.g. by adding spaces on both sides of the * an & in its fixes.

test/clang-tidy/misc-rule-of-five.cpp

1 ↗

(On Diff #46775)

Please add tests with macros and templates with multiple instantiations.

[dblaikie]

Is this really that useful of a rule? The language does the right thing for
the most part already (you don't need to explicitly delete them - they're
implicitly deleted if you define any others - except for backcompat with
C++98, but those cases are deprecated & we should probably split out the
warning for that deprecation so it's easier to turn on separately)

[jbcoe]

If the destructor is user-declared then I need to =delete the compiler-generated copy constructor and copy assignment operator (if they are not defined, if either is defined then they are already handled by this check).

The move constructor and move assignment operator do not suffer from deprecated compiler behaviour so do not need to be explicitly deleted if the destructor is user-declared. Perhaps they should be though as this is called 'rule-of-five' (for now).

I think that deleting special member functions in the presence of a user-defined destructor is the right thing to do even if the user has defined the destructor as =default otherwise the check will go against the intention of the standard.

[jbcoe]

The method I'm using to insert after a function declaration is flawed. Advancing by one character is not always the right thing to do and won't handle cases where there is a space before a semi-colon. I'll add extra tests and see if I can come up with a neater way of handling the post-declaration insertion.

[jbcoe]

This check now deals only with deprecated special member function generation, not move constructors or move assignment operators. I have renamed it and modified documentation accordingly.

Code has been updated in line with review comments.

[jbcoe]

Tests are now more thorough and more readable.

Insertions are always pre-insertions as getting the correct post-insertion position is ambiguous if end-of-line comments exist.

[aaron.ballman]

A few minor nits, but one question (to me) remains: should this be in misc, or is this a cppcoreguideline? I think the check, as is, is acceptable (and can possibly be extended to be a rule-of-five check aliased under cppcoreguidelines), but wanted to understand others' opinions since the categorization has changed a few times.

clang-tidy/misc/DeprecatedSpecialMemberGenerationCheck.cpp

72

Use isa<> instead of dyn_cast<> here since you don't care about the resulting object.

103

No need for FixIt, can just inline buildFixIt below.

[jbcoe]

Minor fixes from review.

[dblaikie]

Is this anything more than the -Wdeprecated warning? (could we split out
the -Wdeprecated warning that deals with the deprecated implicit special
member generation, then just use that warning for this clang-tidy check?)

[jbcoe]

It's more than the warning because it offers fixits. Other than that it should be the same. Using the same code as used to warn on deprecated special members would be a great idea. I'm not too sure where to start looking and how much of Sema is exposed to clang-tidy though.

[jbcoe]

The Sema diagnostic warning is only produced if a deprecated special member function is used whereas I want to find places where it would be compiler-generated and explicitly delete them. This is useful for library code where I don't have control over the warnings my users will run with.

The AST Matcher I use are simple enough and I'm not convinced that it's worth refactoring Sema to expose what I need. If someone (Richard?) with a deeper understanding can point me in the right direction I'm happy to be corrected.

SemaDeclCXX.cpp diagnoseDeprecatedCopyOperation is producing Sema's diagnostics.

[aaron.ballman]

In D16376#360527, @jbcoe wrote:

The Sema diagnostic warning is only produced if a deprecated special member function is used whereas I want to find places where it would be compiler-generated and explicitly delete them. This is useful for library code where I don't have control over the warnings my users will run with.

The AST Matcher I use are simple enough and I'm not convinced that it's worth refactoring Sema to expose what I need. If someone (Richard?) with a deeper understanding can point me in the right direction I'm happy to be corrected.

SemaDeclCXX.cpp diagnoseDeprecatedCopyOperation is producing Sema's diagnostics.

I'm on the fence about whether this functionality should be in both clang-tidy and Sema, but lean towards leaving it separated because of your use case as a library author. However, that suggests perhaps it should be under the modernize umbrella instead of misc because already-modern code shouldn't be using deprecated functionality. Also, the documentation should probably spell out that this differs from warn_deprecated_copy_operation, how it differs, and why that's useful.

[jbcoe]

I'll move this to modernize and update docs when I get over my cold. Thanks for the feedback.

[jbcoe]

After some pondering I think I will extend and move this check to cppcoreguidelines and call it rule-of-five.

https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-default-operation-define-or-delete-them-all

[jbcoe]

I've rewritten this patch to implement a rule-of-five-and-zero check.

No fixes are offered as we've not found them to be useful.

clang-tidy/misc/CMakeLists.txt

   UnusedParametersCheck.cpp
   UnusedRAIICheck.cpp
   UniqueptrResetReleaseCheck.cpp
+  DeprecatedSpecialMemberGenerationCheck.cpp
   VirtualNearMissCheck.cpp
 
   LINK_LIBS

clang-tidy/misc/DeprecatedSpecialMemberGenerationCheck.h

+//===--- DeprecatedSpecialMemberGenerationCheck.h - clang-tidy----*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DEPRECATED_SPECIAL_MEMBER_GENERATION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DEPRECATED_SPECIAL_MEMBER_GENERATION_H
+
+#include "../ClangTidy.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Finds classes where deprecated compiler-generation of special member
+/// functions will generate a copy constructor or copy assignment operator.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-deprecated-special-member-generation.html
+class DeprecatedSpecialMemberGenerationCheck : public ClangTidyCheck {
+public:
+  DeprecatedSpecialMemberGenerationCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  enum class SpecialFunctionKind {
+    CopyConstructor,
+    CopyAssignment,
+    Destructor
+  };
+
+  static std::string
+  buildFixIt(const CXXMethodDecl &MatchedDecl, StringRef ClassName,
+             DeprecatedSpecialMemberGenerationCheck::SpecialFunctionKind S);
+  static StringRef getDiagnosticFormat(
+      DeprecatedSpecialMemberGenerationCheck::SpecialFunctionKind S);
+  void
+  addDiagnosticAndFixIt(const ast_matchers::MatchFinder::MatchResult &Result,
+                        const CXXMethodDecl &MatchedDecl,
+                        SpecialFunctionKind S);
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DEPRECATED_SPECIAL_MEMBER_GENERATION_H

clang-tidy/misc/DeprecatedSpecialMemberGenerationCheck.cpp

+//===--- DeprecatedSpecialMemberGenerationCheck.cpp - clang-tidy-----------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "DeprecatedSpecialMemberGenerationCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void DeprecatedSpecialMemberGenerationCheck::registerMatchers(
+    MatchFinder *Finder) {
+
+  if (!getLangOpts().CPlusPlus)
+    return;
+
+  Finder->addMatcher(
+      cxxRecordDecl(isDefinition(),
+                    has(cxxDestructorDecl(unless(isImplicit())).bind("dtor")),
+                    unless(anyOf(has(cxxConstructorDecl(isCopyConstructor(),
+                                                        unless(isImplicit()))),
+                                 has(cxxMethodDecl(isCopyAssignmentOperator(),
+                                                   unless(isImplicit())))))),
+      this);
+
+  Finder->addMatcher(
+      cxxRecordDecl(isDefinition(), has(cxxConstructorDecl(isCopyConstructor(),
+                                                           unless(isImplicit()))
+                                            .bind("copy-ctor")),
+                    unless(has(cxxMethodDecl(isCopyAssignmentOperator())))),
+      this);
+
+  Finder->addMatcher(
+      cxxRecordDecl(
+          isDefinition(),
+          has(cxxMethodDecl(isCopyAssignmentOperator(), unless(isImplicit()))
+                  .bind("copy assignment")),
+          unless(has(cxxConstructorDecl(isCopyConstructor())))),
+      this);
+}
+
+StringRef DeprecatedSpecialMemberGenerationCheck::getDiagnosticFormat(
+    DeprecatedSpecialMemberGenerationCheck::SpecialFunctionKind S) {
+  switch (S) {
+  case SpecialFunctionKind::Destructor:
+    return "class %0 defines a destructor but not a copy constructor or copy "
+           "assignment operator";
+  case SpecialFunctionKind::CopyConstructor:
+    return "class %0 defines a copy constructor but not a copy assignment "
+           "operator";
+  case SpecialFunctionKind::CopyAssignment:
+    return "class %0 defines a copy assignment operator but not a "
+           "copy constructor";
+  }
+};
+
+std::string DeprecatedSpecialMemberGenerationCheck::buildFixIt(
+    const CXXMethodDecl &MatchedDecl, StringRef ClassName,
+    DeprecatedSpecialMemberGenerationCheck::SpecialFunctionKind S) {
+
+  StringRef DeleteOrDefault =
+      (!isa<CXXDestructorDecl>(&MatchedDecl) && MatchedDecl.isDefaulted())
+          ? "default"
+          : "delete";
+  switch (S) {
+  case SpecialFunctionKind::Destructor:
+    return (llvm::Twine("\n") + ClassName + "(const " + ClassName + " &) = " +
+            DeleteOrDefault + ";\n" + ClassName + " & operator=(const " +
+            ClassName + " &) = " + DeleteOrDefault + ";\n")
+        .str();
+  case SpecialFunctionKind::CopyConstructor:
+    return (llvm::Twine("\n") + ClassName + " & operator=(const " + ClassName +
+            " &) = " + DeleteOrDefault + ";\n")
+        .str();
+  case SpecialFunctionKind::CopyAssignment:
+    return (llvm::Twine("") + ClassName + "(const " + ClassName + " &) = " +
+            DeleteOrDefault + ";\n")
+        .str();
+  }
+};
+
+void DeprecatedSpecialMemberGenerationCheck::addDiagnosticAndFixIt(
+    const MatchFinder::MatchResult &Result, const CXXMethodDecl &MatchedDecl,
+    SpecialFunctionKind S) {
+  const CXXRecordDecl *Class = MatchedDecl.getParent();
+
+  DiagnosticBuilder Diag =
+      diag(MatchedDecl.getLocation(), getDiagnosticFormat(S)) << Class;
+
+  if (!getLangOpts().CPlusPlus11)
+    return;
+
+  Diag << FixItHint::CreateInsertion(
+      MatchedDecl.getLocStart(), buildFixIt(MatchedDecl, Class->getName(), S));
+}
+
+void DeprecatedSpecialMemberGenerationCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  if (const auto *MatchedDecl =
+          Result.Nodes.getNodeAs<CXXDestructorDecl>("dtor"))
+    addDiagnosticAndFixIt(Result, *MatchedDecl,
+                          SpecialFunctionKind::Destructor);
+  else if (const auto *MatchedDecl =
+               Result.Nodes.getNodeAs<CXXConstructorDecl>("copy-ctor"))
+    addDiagnosticAndFixIt(Result, *MatchedDecl,
+                          SpecialFunctionKind::CopyConstructor);
+  else if (const auto *MatchedDecl =
+               Result.Nodes.getNodeAs<CXXMethodDecl>("copy assignment"))
+    addDiagnosticAndFixIt(Result, *MatchedDecl,
+                          SpecialFunctionKind::CopyAssignment);
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang

clang-tidy/misc/MiscTidyModule.cpp

 #include "UnusedAliasDeclsCheck.h"
 #include "UnusedParametersCheck.h"
 #include "UnusedRAIICheck.h"
+#include "DeprecatedSpecialMemberGenerationCheck.h"
 #include "VirtualNearMissCheck.h"
 
 namespace clang {

     CheckFactories.registerCheck<UnusedParametersCheck>(
         "misc-unused-parameters");
     CheckFactories.registerCheck<UnusedRAIICheck>("misc-unused-raii");
+    CheckFactories.registerCheck<DeprecatedSpecialMemberGenerationCheck>(
+        "misc-deprecated-special-member-generation");
     CheckFactories.registerCheck<VirtualNearMissCheck>(
         "misc-virtual-near-miss");
   }

docs/clang-tidy/checks/list.rst

    misc-argument-comment
    misc-assert-side-effect
    misc-assign-operator-signature
+   misc-deprecated-special-member-generation
    misc-bool-pointer-implicit-conversion
    misc-definitions-in-headers
    misc-inaccurate-erase

docs/clang-tidy/checks/misc-deprecated-special-member-generation.rst

+.. title:: clang-tidy - misc-deprecated-special-member-generation
+
+misc-deprecated-special-member-generation
+=========================================
+
+Modern compilers generate copy constructors and copy assignment operators in
+cases where generation of special functions is deprecated by the standard.
+
+C++14 standard [class.copy] paragraph 7:
+
+"If the class definition does not explicitly declare a copy constructor, one is
+declared implicitly. If the class definition declares a move constructor or
+move assignment operator, the implicitly declared copy constructor is defined
+as deleted; otherwise, it is defined as defaulted (8.4). The latter case is
+deprecated if the class has a user-declared copy assignment operator or a
+user-declared destructor."
+
+C++14 standard [class.copy] paragraph 18:
+
+"If the class definition does not explicitly declare a copy assignment
+operator, one is declared implicitly. If the class definition declares a move
+constructor or move assignment operator, the implicitly declared copy
+assignment operator is defined as deleted; otherwise, it is defined as
+defaulted (8.4). The latter case is deprecated if the class has a user-declared
+copy constructor or a user-declared destructor."
+
+This check finds classes where deprecated compiler-generation of special member
+functions may generate a copy constructor or copy assignment operator.
+
+If the destructor is declared, missing copy constructors or copy assignment
+operators are declared '=delete'.
+
+If a copy constructor or assignment operator is declared '=default' and no
+destructor is declared, the missing copy constructor or assignment operator is
+declared '=default'.
+
+Otherwise, if a copy constructor or assignment operator is declared and no
+destructor is declared, the missing copy constructor or assignment operator is
+declared '=delete'.
+

test/clang-tidy/misc-deprecated-special-member-generation.cpp

+// RUN: %check_clang_tidy %s misc-deprecated-special-member-generation %t
+
+//
+// User defined copy constructors
+//
+class DeclaresCopyConstructor {
+  DeclaresCopyConstructor(const DeclaresCopyConstructor &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'DeclaresCopyConstructor' defines a copy constructor but not a copy assignment operator [misc-deprecated-special-member-generation]
+};
+
+// CHECK-FIXES: DeclaresCopyConstructor & operator=(const DeclaresCopyConstructor &) = delete;
+
+class DefinesDefaultedCopyConstructor {
+  DefinesDefaultedCopyConstructor(const DefinesDefaultedCopyConstructor &) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'DefinesDefaultedCopyConstructor' defines a copy constructor but not a copy assignment operator [misc-deprecated-special-member-generation]
+};
+
+// CHECK-FIXES: DefinesDefaultedCopyConstructor & operator=(const DefinesDefaultedCopyConstructor &) = default;
+
+class DefinesDeletedCopyConstructor {
+  DefinesDeletedCopyConstructor(const DefinesDeletedCopyConstructor &) = delete;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'DefinesDeletedCopyConstructor' defines a copy constructor but not a copy assignment operator [misc-deprecated-special-member-generation]
+};
+
+// CHECK-FIXES: DefinesDeletedCopyConstructor & operator=(const DefinesDeletedCopyConstructor &) = delete;
+
+
+//
+// User defined copy assignment
+//
+class DeclaresCopyAssignment {
+  DeclaresCopyAssignment & operator=(const DeclaresCopyAssignment &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: class 'DeclaresCopyAssignment' defines a copy assignment operator but not a copy constructor [misc-deprecated-special-member-generation]
+};
+
+// CHECK-FIXES: DeclaresCopyAssignment(const DeclaresCopyAssignment &) = delete;
+
+class DefinesDefaultedCopyAssignment {
+  DefinesDefaultedCopyAssignment & operator=(const DefinesDefaultedCopyAssignment &) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: class 'DefinesDefaultedCopyAssignment' defines a copy assignment operator but not a copy constructor [misc-deprecated-special-member-generation]
+};
+
+// CHECK-FIXES: DefinesDefaultedCopyAssignment(const DefinesDefaultedCopyAssignment &) = default;
+
+class DefinesDeletedCopyAssignment {
+  DefinesDeletedCopyAssignment & operator=(const DefinesDeletedCopyAssignment &) = delete;
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: class 'DefinesDeletedCopyAssignment' defines a copy assignment operator but not a copy constructor [misc-deprecated-special-member-generation]
+};
+
+// CHECK-FIXES: DefinesDeletedCopyAssignment(const DefinesDeletedCopyAssignment &) = delete;
+
+class DeclaresDestructor {
+  ~DeclaresDestructor();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'DeclaresDestructor' defines a destructor but not a copy constructor or copy assignment operator [misc-deprecated-special-member-generation]
+};
+// CHECK-FIXES: DeclaresDestructor(const DeclaresDestructor &) = delete;
+// CHECK-FIXES: DeclaresDestructor & operator=(const DeclaresDestructor &) = delete;
+
+class DefinesDestructor {
+  ~DefinesDestructor() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'DefinesDestructor' defines a destructor but not a copy constructor or copy assignment operator [misc-deprecated-special-member-generation]
+};
+// CHECK-FIXES: DefinesDestructor(const DefinesDestructor &) = delete;
+// CHECK-FIXES: DefinesDestructor & operator=(const DefinesDestructor &) = delete;
+
+
+class DefinesDefaultedDestructor {
+  ~DefinesDefaultedDestructor() = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'DefinesDefaultedDestructor' defines a destructor but not a copy constructor or copy assignment operator [misc-deprecated-special-member-generation]
+};
+// CHECK-FIXES: DefinesDefaultedDestructor(const DefinesDefaultedDestructor &) = delete;
+// CHECK-FIXES: DefinesDefaultedDestructor & operator=(const DefinesDefaultedDestructor &) = delete;
+
+
+class DefinesDeletedDestructor {
+  ~DefinesDeletedDestructor() = delete;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'DefinesDeletedDestructor' defines a destructor but not a copy constructor or copy assignment operator [misc-deprecated-special-member-generation]
+};
+// CHECK-FIXES: DefinesDeletedDestructor(const DefinesDeletedDestructor &) = delete;
+// CHECK-FIXES: DefinesDeletedDestructor & operator=(const DefinesDeletedDestructor &) = delete;
+
+template <class T>
+class ClassTemplateDeclaresDestructor {
+  ~ClassTemplateDeclaresDestructor();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'ClassTemplateDeclaresDestructor' defines a destructor but not a copy constructor or copy assignment operator [misc-deprecated-special-member-generation]
+};
+// CHECK-FIXES: ClassTemplateDeclaresDestructor(const ClassTemplateDeclaresDestructor &) = delete;
+// CHECK-FIXES: ClassTemplateDeclaresDestructor & operator=(const ClassTemplateDeclaresDestructor &) = delete;
+
+template <>
+class ClassTemplateDeclaresDestructor<int> {
+  ~ClassTemplateDeclaresDestructor();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'ClassTemplateDeclaresDestructor<int>' defines a destructor but not a copy constructor or copy assignment operator [misc-deprecated-special-member-generation]
+};
+// CHECK-FIXES: ClassTemplateDeclaresDestructor(const ClassTemplateDeclaresDestructor &) = delete;
+// CHECK-FIXES: ClassTemplateDeclaresDestructor & operator=(const ClassTemplateDeclaresDestructor &) = delete;
+
+#define DEFAULTED_DESTRUCTOR(X) ~X() = default
+
+class MacroDefaultsDestructor {
+  DEFAULTED_DESTRUCTOR(MacroDefaultsDestructor);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'MacroDefaultsDestructor' defines a destructor but not a copy constructor or copy assignment operator [misc-deprecated-special-member-generation]
+};
+// CHECK-FIXES: MacroDefaultsDestructor(const MacroDefaultsDestructor &) = delete;
+// CHECK-FIXES: MacroDefaultsDestructor & operator=(const MacroDefaultsDestructor &) = delete;
+
+#define VIRTUAL_BASE_CLASS(X) class X { virtual ~X() = default; }
+
+VIRTUAL_BASE_CLASS(VirtualBase);
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: class 'VirtualBase' defines a destructor but not a copy constructor or copy assignment operator [misc-deprecated-special-member-generation]
+
+class DeclaresAllThree {
+  ~DeclaresAllThree();
+  DeclaresAllThree(const DeclaresAllThree&);
+  DeclaresAllThree & operator=(const DeclaresAllThree&);
+};
+
+class DeclaresCopyAndAssignment {
+  DeclaresCopyAndAssignment(const DeclaresCopyAndAssignment&);
+  DeclaresCopyAndAssignment & operator=(const DeclaresCopyAndAssignment&);
+};
+