[PATCH] D13510: [PATCH] Support C++ Core Guidelines copy assignment restrictions

[Aaron Ballman via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: Eugene.Zelenko, alexfh, mgehre, sbenza.
aaron.ballman added a subscriber: cfe-commits.

Eugene pointed out to me that our existing misc-assign-operator-signature check was almost perfectly implementing the C++ Core Guideline guidance for Copy and move, found here: https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-ccopy-copy-and-move. The only part that was missing was ensuring that the operator wasn't marked as virtual.

In this patch, I've implemented the check for the virtual qualifier, and registered the checker as a cppcoreguideline checker in addition to its usual place in misc.

~Aaron

http://reviews.llvm.org/D13510

Files:
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/misc/AssignOperatorSignatureCheck.cpp
  test/clang-tidy/misc-assign-operator-signature.cpp

Index: test/clang-tidy/misc-assign-operator-signature.cpp
===================================================================
--- test/clang-tidy/misc-assign-operator-signature.cpp
+++ test/clang-tidy/misc-assign-operator-signature.cpp
@@ -49,3 +49,8 @@
   // Pre-C++11 way of disabling assignment.
   void operator=(const Private &);
 };
+
+struct Virtual {
+  virtual Virtual& operator=(const Virtual &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should not be marked 'virtual'
+};
Index: clang-tidy/misc/AssignOperatorSignatureCheck.cpp
===================================================================
--- clang-tidy/misc/AssignOperatorSignatureCheck.cpp
+++ clang-tidy/misc/AssignOperatorSignatureCheck.cpp
@@ -51,11 +51,11 @@
           .bind("ArgumentType"),
       this);
 
-  Finder->addMatcher(cxxMethodDecl(IsSelfAssign, isConst()).bind("Const"),
-                     this);
+  Finder->addMatcher(
+      cxxMethodDecl(IsSelfAssign, anyOf(isConst(), isVirtual())).bind("cv"),
+      this);
 }
 
-
 void AssignOperatorSignatureCheck::check(
     const MatchFinder::MatchResult &Result) {
   const auto* Method = Result.Nodes.getNodeAs<CXXMethodDecl>("method");
@@ -64,12 +64,13 @@
   static const char *Messages[][2] = {
       {"ReturnType", "operator=() should return '%0&'"},
       {"ArgumentType", "operator=() should take '%0 const&', '%0&&' or '%0'"},
-      {"Const", "operator=() should not be marked 'const'"},
+      {"cv", "operator=() should not be marked '%1'"}
   };
 
-  for (const auto& Message : Messages) {
+  for (const auto &Message : Messages) {
     if (Result.Nodes.getNodeAs<Decl>(Message[0]))
-      diag(Method->getLocStart(), Message[1]) << Name;
+      diag(Method->getLocStart(), Message[1])
+          << Name << (Method->isConst() ? "const" : "virtual");
   }
 }
 
Index: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===================================================================
--- clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "../misc/AssignOperatorSignatureCheck.h"
 #include "ProTypeConstCastCheck.h"
 #include "ProTypeReinterpretCastCheck.h"
 
@@ -24,6 +25,8 @@
         "cppcoreguidelines-pro-type-const-cast");
     CheckFactories.registerCheck<ProTypeReinterpretCastCheck>(
         "cppcoreguidelines-pro-type-reinterpret-cast");
+    CheckFactories.registerCheck<misc::AssignOperatorSignatureCheck>(
+        "cppcoreguidelines-c-copy-assignment-signature");
   }
 };
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D13510.36734.patch
Type: text/x-patch
Size: 2674 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151007/2597542b/attachment-0001.bin>