jbcoe updated this revision to Diff 45729.
jbcoe added a comment.
Added handling for move-constructor/move-assignment pairs.
http://reviews.llvm.org/D16376
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp
clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.h
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-user-defined-copy-without-assignment.rst
test/clang-tidy/misc-user-defined-copy-without-assignment.cpp
Index: test/clang-tidy/misc-user-defined-copy-without-assignment.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-user-defined-copy-without-assignment.cpp
@@ -0,0 +1,161 @@
+// 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 &&);
+};
Index: docs/clang-tidy/checks/misc-user-defined-copy-without-assignment.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/misc-user-defined-copy-without-assignment.rst
@@ -0,0 +1,36 @@
+.. 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.
+
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -65,6 +65,7 @@
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
Index: clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.h
@@ -0,0 +1,38 @@
+//===--- 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
Index: clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp
@@ -0,0 +1,176 @@
+//===--- 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
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -34,6 +34,7 @@
#include "UnusedAliasDeclsCheck.h"
#include "UnusedParametersCheck.h"
#include "UnusedRAIICheck.h"
+#include "UserDefinedCopyWithoutAssignmentCheck.h"
#include "VirtualNearMissCheck.h"
namespace clang {
@@ -88,6 +89,8 @@
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");
}
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -26,6 +26,7 @@
UnusedParametersCheck.cpp
UnusedRAIICheck.cpp
UniqueptrResetReleaseCheck.cpp
+ UserDefinedCopyWithoutAssignmentCheck.cpp
VirtualNearMissCheck.cpp
LINK_LIBS
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
