[PATCH] D80490: Check for rule of five and zero.
vrnithinkumar updated this revision to Diff 265921.
vrnithinkumar added a comment.
fixed the clang-tidy warnig
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80490/new/
https://reviews.llvm.org/D80490
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-rule-of-five-and-zero.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-rule-of-five-and-zero-strict.cpp
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-rule-of-five-and-zero.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-rule-of-five-and-zero.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-rule-of-five-and-zero.cpp
@@ -0,0 +1,122 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-rule-of-five-and-zero %t
+
+class DefinesNothing {
+};
+
+class DefinesEverything {
+ DefinesEverything(const DefinesEverything &);
+ DefinesEverything =(const DefinesEverything &);
+ DefinesEverything(DefinesEverything &&);
+ DefinesEverything =(DefinesEverything &&);
+ ~DefinesEverything();
+};
+
+class DeletesEverything {
+ DeletesEverything(const DeletesEverything &);
+ DeletesEverything =(const DeletesEverything &);
+ DeletesEverything(DeletesEverything &&);
+ DeletesEverything =(DeletesEverything &&);
+ ~DeletesEverything();
+};
+
+// Safe cases
+class DefinesAllExceptMoves {
+ ~DefinesAllExceptMoves();
+ DefinesAllExceptMoves(DefinesAllExceptMoves &);
+ DefinesAllExceptMoves =(DefinesAllExceptMoves &);
+};
+
+class DefinesAllExceptMoveConstructor {
+ ~DefinesAllExceptMoveConstructor();
+ DefinesAllExceptMoveConstructor(DefinesAllExceptMoveConstructor &);
+ DefinesAllExceptMoveConstructor =(DefinesAllExceptMoveConstructor &);
+ DefinesAllExceptMoveConstructor =(DefinesAllExceptMoveConstructor &&);
+};
+
+class DefinesAllExceptMoveAssignment {
+ ~DefinesAllExceptMoveAssignment();
+ DefinesAllExceptMoveAssignment(DefinesAllExceptMoveAssignment &);
+ DefinesAllExceptMoveAssignment =(DefinesAllExceptMoveAssignment &);
+ DefinesAllExceptMoveAssignment(DefinesAllExceptMoveAssignment &&);
+};
+
+class DefinesDestructorAndMoveConstructor {
+ ~DefinesDestructorAndMoveConstructor();
+ DefinesDestructorAndMoveConstructor(DefinesDestructorAndMoveConstructor &&);
+};
+
+class DefinesDestructorAndMoveAssignment {
+ ~DefinesDestructorAndMoveAssignment();
+ DefinesDestructorAndMoveAssignment =(DefinesDestructorAndMoveAssignment &&);
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:7: warning: class 'DefinesOnlyDestructor' defines a destructor but does not define all other 5, define all other 5 or delete all other special member functions [cppcoreguidelines-rule-of-five-and-zero]
+class DefinesOnlyDestructor {
+ ~DefinesOnlyDestructor();
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:7: warning: class 'DefinesOnlyCopyConstructor' defines a copy constructor but does not define all other 5, define all other 5 or delete all other special member functions [cppcoreguidelines-rule-of-five-and-zero]
+class DefinesOnlyCopyConstructor {
+ DefinesOnlyCopyConstructor(const DefinesOnlyCopyConstructor &);
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:7: warning: class 'DefinesOnlyMoveConstructor' defines a move constructor but does not define all other 5, define all other 5 or delete all other special member functions [cppcoreguidelines-rule-of-five-and-zero]
+class DefinesOnlyMoveConstructor {
+ DefinesOnlyMoveConstructor(DefinesOnlyMoveConstructor &&);
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:7: warning: class 'DefinesOnlyCopyAssignment' defines a copy assignment operator but does not define all other 5, define all other 5 or delete all other special member functions [cppcoreguidelines-rule-of-five-and-zero]
+class DefinesOnlyCopyAssignment {
+ DefinesOnlyCopyAssignment =(const DefinesOnlyCopyAssignment &);
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:7: warning: class 'DefinesOnlyMoveAssignment' defines a move assignment operator but does not define all other 5, define all other 5 or delete all other special member functions [cppcoreguidelines-rule-of-five-and-zero]
+class DefinesOnlyMoveAssignment {
+ DefinesOnlyMoveAssignment =(DefinesOnlyMoveAssignment &&);
+};
+
+// check inheritance
+class NoCopyBase {
+public:
+ ~NoCopyBase();
+ NoCopyBase(NoCopyBase &&);
+ NoCopyBase =(NoCopyBase &&);
+
+private:
+ NoCopyBase(const NoCopyBase &) = delete;
+ NoCopyBase =(const NoCopyBase &) = delete;
+};
+
+class NoMoveBase {
+public:
+ ~NoMoveBase();
+ NoMoveBase(const NoMoveBase &);
+ NoMoveBase
[PATCH] D80490: Check for rule of five and zero.
vrnithinkumar created this revision.
vrnithinkumar added reviewers: aaron.ballman, alexfh, jbcoe, dblaikie, rsmith.
Herald added subscribers: cfe-commits, kbarton, mgorny, nemanjai.
Herald added a project: clang.
New check to check if a class defines all special members of none of them. This
also known as rule of five and zero.
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.
In summary the check will
- Checks class defines all special members of none of them.
- It also see if the base class deletes the special members or not.
- Has two modes with the flag strict-check.
- Strict mode will check all or nothing strictly.
For some combination, compiler explicitly delete the special members or does
not declare implicitly. In that case don'nt have to define all the special
members.
For example, in case of defining all members except move constructor and move
assignment compiler will not implicitly declare the move constructor and move
assignment.
For non strict mode we will consider these combinations as safe.
I found one review https://reviews.llvm.org/D16376 already related to this.
I modified my changes based on this. But I could not find out why this got
abandoned.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D80490
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-rule-of-five-and-zero.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-rule-of-five-and-zero-strict.cpp
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-rule-of-five-and-zero.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-rule-of-five-and-zero.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-rule-of-five-and-zero.cpp
@@ -0,0 +1,122 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-rule-of-five-and-zero %t
+
+class DefinesNothing {
+};
+
+class DefinesEverything {
+ DefinesEverything(const DefinesEverything &);
+ DefinesEverything =(const DefinesEverything &);
+ DefinesEverything(DefinesEverything &&);
+ DefinesEverything =(DefinesEverything &&);
+ ~DefinesEverything();
+};
+
+class DeletesEverything {
+ DeletesEverything(const DeletesEverything &);
+ DeletesEverything =(const DeletesEverything &);
+ DeletesEverything(DeletesEverything &&);
+ DeletesEverything =(DeletesEverything &&);
+ ~DeletesEverything();
+};
+
+// Safe cases
+class DefinesAllExceptMoves {
+ ~DefinesAllExceptMoves();
+ DefinesAllExceptMoves(DefinesAllExceptMoves &);
+ DefinesAllExceptMoves =(DefinesAllExceptMoves &);
+};
+
+class DefinesAllExceptMoveConstructor {
+ ~DefinesAllExceptMoveConstructor();
+ DefinesAllExceptMoveConstructor(DefinesAllExceptMoveConstructor &);
+ DefinesAllExceptMoveConstructor =(DefinesAllExceptMoveConstructor &);
+ DefinesAllExceptMoveConstructor =(DefinesAllExceptMoveConstructor &&);
+};
+
+class DefinesAllExceptMoveAssignment {
+ ~DefinesAllExceptMoveAssignment();
+ DefinesAllExceptMoveAssignment(DefinesAllExceptMoveAssignment &);
+ DefinesAllExceptMoveAssignment =(DefinesAllExceptMoveAssignment &);
+ DefinesAllExceptMoveAssignment(DefinesAllExceptMoveAssignment &&);
+};
+
+class DefinesDestructorAndMoveConstructor {
+ ~DefinesDestructorAndMoveConstructor();
+ DefinesDestructorAndMoveConstructor(DefinesDestructorAndMoveConstructor &&);
+};
+
+class DefinesDestructorAndMoveAssignment {
+ ~DefinesDestructorAndMoveAssignment();
+ DefinesDestructorAndMoveAssignment =(DefinesDestructorAndMoveAssignment &&);
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:7: warning: class 'DefinesOnlyDestructor' defines a destructor but does not define all other 5, define all other 5 or delete all other special member functions [cppcoreguidelines-rule-of-five-and-zero]
+class DefinesOnlyDestructor {
+ ~DefinesOnlyDestructor();
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:7: warning: class 'DefinesOnlyCopyConstructor' defines a copy constructor but does not define all other 5, define all other 5 or delete all other special member functions [cppcoreguidelines-rule-of-five-and-zero]
+class DefinesOnlyCopyConstructor {
+ DefinesOnlyCopyConstructor(const DefinesOnlyCopyConstructor &);
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:7: warning: class 'DefinesOnlyMoveConstructor' defines a move constructor but does not define all other 5, define all other 5 or delete all other special member functions [cppcoreguidelines-rule-of-five-and-zero]
