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");
}
};
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");
}
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
