jbcoe retitled this revision from "clang-tidy check: User-defined copy without
assignment" to "clang-tidy check: Assignment and Construction".
jbcoe updated the summary for this revision.
jbcoe set the repository for this revision to rL LLVM.
jbcoe updated this revision to Diff 45987.
jbcoe added a comment.
Rename check and restructure code to avoid repetition.
Repository:
rL LLVM
http://reviews.llvm.org/D16376
Files:
clang-tidy/misc/AssignmentAndConstructionCheck.cpp
clang-tidy/misc/AssignmentAndConstructionCheck.h
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-assignment-and-construction.rst
test/clang-tidy/misc-assignment-and-construction.cpp
Index: test/clang-tidy/misc-assignment-and-construction.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-assignment-and-construction.cpp
@@ -0,0 +1,161 @@
+// RUN: %check_clang_tidy %s misc-assignment-and-construction %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-assignment-and-construction]
+};
+
+// 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-assignment-and-construction]
+};
+
+// 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-assignment-and-construction]
+};
+
+// 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-assignment-and-construction]
+};
+
+// 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-assignment-and-construction]
+};
+
+// 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-assignment-and-construction]
+};
+
+// 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-assignment-and-construction]
+};
+
+// 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-assignment-and-construction]
+};
+
+// 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-assignment-and-construction.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/misc-assignment-and-construction.rst
@@ -0,0 +1,25 @@
+.. title:: clang-tidy - misc-assignment-and-construction
+
+misc-assignment-and-construction
+=========================================
+
+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 [class.construction 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 only one of the copy/move constructor and
+copy/move assignment operator is user-defined. The fix is defensive. Incorrect
+compiler-generated assignment/construction can cause unexpected behaviour. An
+explicitly deleted constructor or assignment operator will cause a compiler
+error if it is used.
+
+Where the user has defaulted one of the pair, the other will be explictly
+defaulted.
+
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-assignment-and-construction
misc-virtual-near-miss
modernize-loop-convert
modernize-make-unique
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 "AssignmentAndConstructionCheck.h"
#include "VirtualNearMissCheck.h"
namespace clang {
@@ -88,6 +89,8 @@
CheckFactories.registerCheck<UnusedParametersCheck>(
"misc-unused-parameters");
CheckFactories.registerCheck<UnusedRAIICheck>("misc-unused-raii");
+ CheckFactories.registerCheck<AssignmentAndConstructionCheck>(
+ "misc-assignment-and-construction");
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
+ AssignmentAndConstructionCheck.cpp
VirtualNearMissCheck.cpp
LINK_LIBS
Index: clang-tidy/misc/AssignmentAndConstructionCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/AssignmentAndConstructionCheck.h
@@ -0,0 +1,48 @@
+//===--- AssignmentAndConstructionCheck.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_ASSIGNMENT_AND_CONSTRUCTION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_ASSIGNMENT_AND_CONSTRUCTION_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Finds copy/move constructors without a matching copy/move assignment operator
+/// or copy/move assignment operators without a matching copy/move constructor.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-assignment-and-construction.html
+class AssignmentAndConstructionCheck : public ClangTidyCheck {
+public:
+ AssignmentAndConstructionCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+ enum class SpecialFunctionKind {
+ CopyConstructor,
+ CopyAssignment,
+ MoveConstructor,
+ MoveAssignment
+ };
+
+private:
+ void diagnosticAndFixIt(
+ 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_ASSIGNMENT_AND_CONSTRUCTION_H
Index: clang-tidy/misc/AssignmentAndConstructionCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/AssignmentAndConstructionCheck.cpp
@@ -0,0 +1,174 @@
+//===--- AssignmentAndConstructionCheck.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 "AssignmentAndConstructionCheck.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 AssignmentAndConstructionCheck::registerMatchers(MatchFinder *Finder) {
+
+ if (!getLangOpts().CPlusPlus)
+ return;
+
+ 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);
+
+ if (!getLangOpts().CPlusPlus11)
+ return;
+
+ 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";
+}
+
+static SourceLocation
+getInsertionLocation(const CXXMethodDecl &MatchedDecl,
+ const MatchFinder::MatchResult &Result,
+ AssignmentAndConstructionCheck::SpecialFunctionKind S) {
+ switch (S) {
+ case AssignmentAndConstructionCheck::SpecialFunctionKind::CopyConstructor:
+ case AssignmentAndConstructionCheck::SpecialFunctionKind::MoveConstructor: {
+ SourceLocation DeclEnd = Lexer::getLocForEndOfToken(
+ MatchedDecl.getLocEnd(), 0, *Result.SourceManager,
+ Result.Context->getLangOpts());
+ return DeclEnd.getLocWithOffset(1);
+ }
+ case AssignmentAndConstructionCheck::SpecialFunctionKind::CopyAssignment:
+ case AssignmentAndConstructionCheck::SpecialFunctionKind::MoveAssignment: {
+ SourceLocation DeclBegin = MatchedDecl.getLocStart();
+ return DeclBegin.getLocWithOffset(-1);
+ }
+ }
+}
+
+StringRef diagnosticFormat(AssignmentAndConstructionCheck::SpecialFunctionKind S) {
+ switch (S) {
+ case AssignmentAndConstructionCheck::SpecialFunctionKind::CopyConstructor:
+ return "class '%0' defines a copy-constructor but not a copy-assignment "
+ "operator";
+ case AssignmentAndConstructionCheck::SpecialFunctionKind::CopyAssignment:
+ return "class '%0' defines a copy-assignment operator but not a "
+ "copy-constructor";
+ case AssignmentAndConstructionCheck::SpecialFunctionKind::MoveConstructor:
+ return "class '%0' defines a move-constructor but not a move-assignment "
+ "operator";
+ case AssignmentAndConstructionCheck::SpecialFunctionKind::MoveAssignment:
+ return "class '%0' defines a move-assignment operator but not a "
+ "move-constructor";
+ }
+};
+
+static std::string buildFixIt(const CXXMethodDecl &MatchedDecl, StringRef ClassName,
+ AssignmentAndConstructionCheck::SpecialFunctionKind S) {
+ switch (S) {
+ case AssignmentAndConstructionCheck::SpecialFunctionKind::CopyConstructor:
+ return (llvm::Twine("\n") + ClassName + " &operator=(const " + ClassName +
+ " &) = " + deleteOrDefault(MatchedDecl) + ";")
+ .str();
+ case AssignmentAndConstructionCheck::SpecialFunctionKind::CopyAssignment:
+ return (llvm::Twine("") + ClassName + "(const " + ClassName + " &) = " +
+ deleteOrDefault(MatchedDecl) + ";\n")
+ .str();
+ case AssignmentAndConstructionCheck::SpecialFunctionKind::MoveConstructor:
+ return (llvm::Twine("\n") + ClassName + " &operator=(" + ClassName +
+ " &&) = " + deleteOrDefault(MatchedDecl) + ";")
+ .str();
+ case AssignmentAndConstructionCheck::SpecialFunctionKind::MoveAssignment:
+ return (llvm::Twine("") + ClassName + "(" + ClassName + " &&) = " +
+ deleteOrDefault(MatchedDecl) + ";\n")
+ .str();
+ }
+};
+
+void AssignmentAndConstructionCheck::diagnosticAndFixIt(
+ const MatchFinder::MatchResult &Result, const CXXMethodDecl &MatchedDecl,
+ SpecialFunctionKind S) {
+ StringRef ClassName = MatchedDecl.getParent()->getName();
+
+ DiagnosticBuilder Diag = diag(MatchedDecl.getLocation(), diagnosticFormat(S))
+ << ClassName;
+
+ if (!getLangOpts().CPlusPlus11)
+ return;
+
+ SourceLocation FixitLocation = getInsertionLocation(MatchedDecl, Result, S);
+ if (FixitLocation.isInvalid())
+ return;
+
+ std::string FixIt = buildFixIt(MatchedDecl, ClassName, S); (llvm::Twine("\n") + ClassName + " &operator=(const " +
+ ClassName + " &) = " + deleteOrDefault(MatchedDecl) + ";")
+ .str();
+
+ Diag << FixItHint::CreateInsertion(FixitLocation, FixIt);
+}
+
+void AssignmentAndConstructionCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ if (const auto *MatchedDecl =
+ Result.Nodes.getNodeAs<CXXConstructorDecl>("copy-ctor")) {
+ diagnosticAndFixIt(Result, *MatchedDecl,
+ SpecialFunctionKind::CopyConstructor);
+ } else if (const auto *MatchedDecl =
+ Result.Nodes.getNodeAs<CXXMethodDecl>("copy-assignment")) {
+ diagnosticAndFixIt(Result, *MatchedDecl,
+ SpecialFunctionKind::CopyAssignment);
+ }
+
+ else if (const auto *MatchedDecl =
+ Result.Nodes.getNodeAs<CXXConstructorDecl>("move-ctor")) {
+ diagnosticAndFixIt(Result, *MatchedDecl,
+ SpecialFunctionKind::MoveConstructor);
+ } else if (const auto *MatchedDecl =
+ Result.Nodes.getNodeAs<CXXMethodDecl>("move-assignment")) {
+ diagnosticAndFixIt(Result, *MatchedDecl,
+ SpecialFunctionKind::MoveAssignment);
+ }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
