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

52

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

38

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

50

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

clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.h

25

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

25

s/assigneemnt/assignment

[alexfh]

clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp

33

This check should be done in the matcher.

50

+1 for operator=

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

52

Just insert this at the start of registerMatchers:

if (!getLangOpts().CPlusPlus11)
  return;

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

7

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

[aaron.ballman]

clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp

52

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

7

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

7

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

24

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

26

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

25

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

26

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

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

[aaron.ballman]

clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp

22

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

63

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

clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.h

21

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

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

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

71 ↗

(On Diff #47735)

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

102 ↗

(On Diff #47735)

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
+  UserDefinedCopyWithoutAssignmentCheck.cpp
   VirtualNearMissCheck.cpp
 
   LINK_LIBS

clang-tidy/misc/MiscTidyModule.cpp

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

     CheckFactories.registerCheck<UnusedParametersCheck>(
         "misc-unused-parameters");
     CheckFactories.registerCheck<UnusedRAIICheck>("misc-unused-raii");
+    CheckFactories.registerCheck<UserDefinedCopyWithoutAssignmentCheck>(
+        "misc-user-defined-copy-without-assignment");
     CheckFactories.registerCheck<VirtualNearMissCheck>(
         "misc-virtual-near-miss");
   }

clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.h

+//===--- UserDefinedCopyWithoutAssignmentCheck.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_USER_DEFINED_COPY_WITHOUT_ASSIGNMENT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USER_DEFINED_COPY_WITHOUT_ASSIGNMENT_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Finds user-defined copy-constructors but no assignement operator.
+///
+/// MSVC 2015 will generate an assignment operator even if the user defines a
+/// copy-constructor. This check finds classes with user-defined
+/// copy-constructors but no assignement operator and (defensively) defines the
+/// assignment operator to be `= delete`.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-user-defined-copy-without-assignment.html
+class UserDefinedCopyWithoutAssignmentCheck : public ClangTidyCheck { public:
+  UserDefinedCopyWithoutAssignmentCheck(StringRef Name, ClangTidyContext
+      *Context) : ClangTidyCheck(Name, Context) {} void
+    registerMatchers(ast_matchers::MatchFinder *Finder) override; void
+    check(const ast_matchers::MatchFinder::MatchResult &Result) override; };
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USER_DEFINED_COPY_WITHOUT_ASSIGNMENT_H

clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp

+//===--- UserDefinedCopyWithoutAssignmentCheck.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 "UserDefinedCopyWithoutAssignmentCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void UserDefinedCopyWithoutAssignmentCheck::registerMatchers(
+    MatchFinder *Finder) {
+  Finder->addMatcher(
+      cxxRecordDecl(
+          isDefinition(), hasDescendant(cxxConstructorDecl(isCopyConstructor(),
+                                                           unless(isImplicit()))
+                                            .bind("copy-ctor")),
+          unless(hasDescendant(cxxMethodDecl(isCopyAssignmentOperator())))),
+      this);
+
+  Finder->addMatcher(
+      cxxRecordDecl(
+          isDefinition(),
+          hasDescendant(
+              cxxMethodDecl(isCopyAssignmentOperator(), unless(isImplicit()))
+                  .bind("copy-assignment")),
+          unless(hasDescendant(cxxConstructorDecl(isCopyConstructor())))),
+      this);
+
+  Finder->addMatcher(
+      cxxRecordDecl(
+          isDefinition(), hasDescendant(cxxConstructorDecl(isMoveConstructor(),
+                                                           unless(isImplicit()))
+                                            .bind("move-ctor")),
+          unless(hasDescendant(cxxMethodDecl(isMoveAssignmentOperator())))),
+      this);
+
+  Finder->addMatcher(
+      cxxRecordDecl(
+          isDefinition(),
+          hasDescendant(
+              cxxMethodDecl(isMoveAssignmentOperator(), unless(isImplicit()))
+                  .bind("move-assignment")),
+          unless(hasDescendant(cxxConstructorDecl(isMoveConstructor())))),
+      this);
+}
+
+static StringRef deleteOrDefault(const CXXMethodDecl *MethodDecl) {
+  return MethodDecl->isDefaulted() ? "default" : "delete";
+}
+
+void UserDefinedCopyWithoutAssignmentCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  if (const auto *MatchedDecl =
+          Result.Nodes.getNodeAs<CXXConstructorDecl>("copy-ctor")) {
+    //
+    // Copy-constructor
+    //
+    StringRef ClassName = MatchedDecl->getParent()->getName();
+
+    DiagnosticBuilder Diag = diag(MatchedDecl->getLocation(),
+                                  "class '%0' defines a copy-constructor "
+                                  "but not a copy-assignment operator")
+                             << ClassName;
+
+    if (!getLangOpts().CPlusPlus11)
+      return;
+
+    SourceLocation CCtorEnd = Lexer::getLocForEndOfToken(
+        MatchedDecl->getLocEnd(), 0, *Result.SourceManager,
+        Result.Context->getLangOpts());
+    CCtorEnd = CCtorEnd.getLocWithOffset(1);
+    if (CCtorEnd.isInvalid())
+      return;
+
+    auto Insertion = (llvm::Twine("\n") + ClassName + " &operator=(const " +
+                      ClassName + " &) = " + deleteOrDefault(MatchedDecl) + ";")
+                         .str();
+
+    Diag << FixItHint::CreateInsertion(CCtorEnd, Insertion);
+  } else if (const auto *MatchedDecl =
+                 Result.Nodes.getNodeAs<CXXMethodDecl>("copy-assignment")) {
+    //
+    // Copy-assignment
+    //
+    StringRef ClassName = MatchedDecl->getParent()->getName();
+
+    DiagnosticBuilder Diag =
+        diag(MatchedDecl->getLocation(), "class '%0' defines a copy-assignment "
+                                         "operator but not a copy-constructor")
+        << ClassName;
+
+    if (!getLangOpts().CPlusPlus11)
+      return;
+
+    SourceLocation AssignmentBegin = MatchedDecl->getLocStart();
+    auto BeforeAssignment = AssignmentBegin.getLocWithOffset(-1);
+    if (BeforeAssignment.isInvalid())
+      return;
+
+    auto Insertion = (llvm::Twine("") + ClassName + "(const " + ClassName +
+                      " &) = " + deleteOrDefault(MatchedDecl) + ";\n")
+                         .str();
+
+    Diag << FixItHint::CreateInsertion(BeforeAssignment, Insertion);
+  }
+
+  else if (const auto *MatchedDecl =
+               Result.Nodes.getNodeAs<CXXConstructorDecl>("move-ctor")) {
+    //
+    // Move-ctor
+    //
+    StringRef ClassName = MatchedDecl->getParent()->getName();
+
+    DiagnosticBuilder Diag = diag(MatchedDecl->getLocation(),
+                                  "class '%0' defines a move-constructor "
+                                  "but not a move-assignment operator")
+                             << ClassName;
+
+    if (!getLangOpts().CPlusPlus11)
+      return;
+
+    SourceLocation CCtorEnd = Lexer::getLocForEndOfToken(
+        MatchedDecl->getLocEnd(), 0, *Result.SourceManager,
+        Result.Context->getLangOpts());
+    CCtorEnd = CCtorEnd.getLocWithOffset(1);
+    if (CCtorEnd.isInvalid())
+      return;
+
+    auto Insertion =
+        (llvm::Twine("\n") + ClassName + " &operator=(" + ClassName +
+         " &&) = " + deleteOrDefault(MatchedDecl) + ";")
+            .str();
+
+    Diag << FixItHint::CreateInsertion(CCtorEnd, Insertion);
+  } else if (const auto *MatchedDecl =
+                 Result.Nodes.getNodeAs<CXXMethodDecl>("move-assignment")) {
+    //
+    // Move-assignment
+    //
+    StringRef ClassName = MatchedDecl->getParent()->getName();
+
+    DiagnosticBuilder Diag =
+        diag(MatchedDecl->getLocation(), "class '%0' defines a move-assignment "
+                                         "operator but not a move-constructor")
+        << ClassName;
+
+    if (!getLangOpts().CPlusPlus11)
+      return;
+
+    SourceLocation AssignmentBegin = MatchedDecl->getLocStart();
+    auto BeforeAssignment = AssignmentBegin.getLocWithOffset(-1);
+    if (BeforeAssignment.isInvalid())
+      return;
+
+    auto Insertion = (llvm::Twine("") + ClassName + "(" + ClassName +
+                      " &&) = " + deleteOrDefault(MatchedDecl) + ";\n")
+                         .str();
+
+    Diag << FixItHint::CreateInsertion(BeforeAssignment, Insertion);
+  }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang

docs/clang-tidy/checks/list.rst

    misc-unused-alias-decls
    misc-unused-parameters
    misc-unused-raii
+   misc-user-defined-copy-without-assignment
    misc-virtual-near-miss
    modernize-loop-convert
    modernize-make-unique

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

+.. title:: clang-tidy - misc-user-defined-copy-without-assignment
+
+misc-user-defined-copy-without-assignment
+=========================================
+
+Compilers will generate an assignment operator even if the user defines a copy
+constructor.  This behaviour is deprecated by the standard (C++ 14 draft
+standard 12.8.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 with a user-defined (including deleted)
+copy-constructor but no assignment operator.
+
+  .. code:: c++
+    class A {
+      A(const A&);
+    };
+
+Will be matched and fixed to delete the assignment operator:
+
+  .. code:: c++
+    class A {
+      A(const A&);
+      A& operator = (const A&) = delete;
+    };
+
+The fix is defensive. Incorrect compiler-generated assignement can cause
+unexpected behaviour. An explicitly deleted assignment operator will cause a
+compiler error if it is used.
+

test/clang-tidy/misc-user-defined-copy-without-assignment.cpp

+// RUN: %check_clang_tidy %s misc-user-defined-copy-without-assignment %t
+
+//
+// User defined copy-constructors
+//
+class A {
+  A(const A &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'A' defines a copy-constructor but not a copy-assignment operator [misc-user-defined-copy-without-assignment]
+};
+
+// CHECK-FIXES: class A {
+// CHECK-FIXES-NEXT: A(const A &);
+// CHECK-FIXES-NEXT: A &operator=(const A &) = delete;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class B {
+  B(const B &);
+  B &operator=(const B &);
+};
+
+class C {
+  C(const C &) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'C' defines a copy-constructor but not a copy-assignment operator [misc-user-defined-copy-without-assignment]
+};
+
+// CHECK-FIXES: class C {
+// CHECK-FIXES-NEXT: C(const C &) = default;
+// CHECK-FIXES-NEXT: C &operator=(const C &) = default;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class D {
+  D(const D &);
+  D &operator=(const D &) = default;
+};
+
+class E {
+  E(const E &);
+  E &operator=(const E &) = delete;
+};
+
+//
+// User defined copy-assignment
+//
+class A2 {
+  A2 &operator=(const A2 &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class 'A2' defines a copy-assignment operator but not a copy-constructor [misc-user-defined-copy-without-assignment]
+};
+
+// CHECK-FIXES: class A2 {
+// CHECK-FIXES-NEXT: A2(const A2 &) = delete;
+// CHECK-FIXES-NEXT: A2 &operator=(const A2 &);
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class B2 {
+  B2(const B2 &);
+  B2 &operator=(const B2 &);
+};
+
+class C2 {
+  C2 &operator=(const C2 &) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class 'C2' defines a copy-assignment operator but not a copy-constructor [misc-user-defined-copy-without-assignment]
+};
+
+// CHECK-FIXES: class C2 {
+// CHECK-FIXES-NEXT: C2(const C2 &) = default;
+// CHECK-FIXES-NEXT: C2 &operator=(const C2 &) = default;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class D2 {
+  D2(const D2 &) = default;
+  D2 &operator=(const D2 &);
+};
+
+class E2 {
+  E2(const E2 &) = delete;
+  E2 &operator=(const E2 &);
+};
+
+//
+// User defined move-constructors
+//
+class A3 {
+  A3(A3 &&);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'A3' defines a move-constructor but not a move-assignment operator [misc-user-defined-copy-without-assignment]
+};
+
+// CHECK-FIXES: class A3 {
+// CHECK-FIXES-NEXT: A3(A3 &&);
+// CHECK-FIXES-NEXT: A3 &operator=(A3 &&) = delete;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class B3 {
+  B3(B3 &&);
+  B3 &operator=(B3 &&);
+};
+
+class C3 {
+  C3(C3 &&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'C3' defines a move-constructor but not a move-assignment operator [misc-user-defined-copy-without-assignment]
+};
+
+// CHECK-FIXES: class C3 {
+// CHECK-FIXES-NEXT: C3(C3 &&) = default;
+// CHECK-FIXES-NEXT: C3 &operator=(C3 &&) = default;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class D3 {
+  D3(D3 &&);
+  D3 &operator=(D3 &&) = default;
+};
+
+class E3 {
+  E3(E3 &&);
+  E3 &operator=(E3 &&) = delete;
+};
+
+//
+// User defined move-assignment
+//
+class A4 {
+  A4 &operator=(A4 &&);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class 'A4' defines a move-assignment operator but not a move-constructor [misc-user-defined-copy-without-assignment]
+};
+
+// CHECK-FIXES: class A4 {
+// CHECK-FIXES-NEXT: A4(A4 &&) = delete;
+// CHECK-FIXES-NEXT: A4 &operator=(A4 &&);
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class B4 {
+  B4(B4 &&);
+  B4 &operator=(B4 &&);
+};
+
+class C4 {
+  C4 &operator=(C4 &&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class 'C4' defines a move-assignment operator but not a move-constructor [misc-user-defined-copy-without-assignment]
+};
+
+// CHECK-FIXES: class C4 {
+// CHECK-FIXES-NEXT: C4(C4 &&) = default;
+// CHECK-FIXES-NEXT: C4 &operator=(C4 &&) = default;
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: };
+
+class D4 {
+  D4(D4 &&) = default;
+  D4 &operator=(D4 &&);
+};
+
+class E4 {
+  E4(E4 &&) = delete;
+  E4 &operator=(E4 &&);
+};