baloghadamsoftware updated this revision to Diff 51906.
baloghadamsoftware added a comment.
Requested fixes done (not related to the changes).
http://reviews.llvm.org/D18265
Files:
clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tidy/misc/AssignOperatorCheck.cpp
clang-tidy/misc/AssignOperatorCheck.h
clang-tidy/misc/AssignOperatorSignatureCheck.cpp
clang-tidy/misc/AssignOperatorSignatureCheck.h
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-assign-operator-signature.rst
docs/clang-tidy/checks/misc-assign-operator.rst
test/clang-tidy/misc-assign-operator-signature.cpp
test/clang-tidy/misc-assign-operator.cpp
Index: test/clang-tidy/misc-assign-operator.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-assign-operator.cpp
@@ -0,0 +1,84 @@
+// RUN: %check_clang_tidy %s misc-assign-operator %t -- -- -std=c++11 -isystem %S/Inputs/Headers
+
+#include <utility>
+
+struct Good {
+ Good& operator=(const Good&);
+ Good& operator=(Good&&);
+
+ // Assign from other types is fine too.
+ Good& operator=(int);
+};
+
+struct AlsoGood {
+ // By value is also fine.
+ AlsoGood& operator=(AlsoGood);
+};
+
+struct BadReturnType {
+ void operator=(const BadReturnType&);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should return 'BadReturnType&' [misc-assign-operator]
+ const BadReturnType& operator=(BadReturnType&&);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should return 'Bad
+ void operator=(int);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should return 'Bad
+};
+
+struct BadReturnType2 {
+ BadReturnType2&& operator=(const BadReturnType2&);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should return 'Bad
+ int operator=(BadReturnType2&&);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should return 'Bad
+};
+
+struct BadArgument {
+ BadArgument& operator=(BadArgument&);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should take 'BadArgument const&', 'BadArgument&&' or 'BadArgument'
+ BadArgument& operator=(const BadArgument&&);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should take 'BadAr
+};
+
+struct BadModifier {
+ BadModifier& operator=(const BadModifier&) const;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should not be marked 'const'
+};
+
+struct Deleted {
+ // We don't check the return value of deleted operators.
+ void operator=(const Deleted&) = delete;
+ void operator=(Deleted&&) = delete;
+};
+
+class Private {
+ // We don't check the return value of private operators.
+ // 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'
+};
+
+class BadReturnStatement {
+ int n;
+
+public:
+ BadReturnStatement& operator=(const BadReturnStatement& rhs) {
+ n = rhs.n;
+ return *this;
+ }
+
+ BadReturnStatement& operator=(BadReturnStatement&& rhs) {
+ n = std::move(rhs.n);
+ return rhs;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operator=() should always return '*this'
+ }
+
+ // Do not check if return type is different from '&BadReturnStatement'
+ int operator=(int i) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should return 'Bad
+ n = i;
+ return n;
+ }
+};
Index: test/clang-tidy/misc-assign-operator-signature.cpp
===================================================================
--- test/clang-tidy/misc-assign-operator-signature.cpp
+++ /dev/null
@@ -1,58 +0,0 @@
-// RUN: %check_clang_tidy %s misc-assign-operator-signature %t
-
-struct Good {
- Good& operator=(const Good&);
- Good& operator=(Good&&);
-
- // Assign from other types is fine too.
- Good& operator=(int);
-};
-
-struct AlsoGood {
- // By value is also fine.
- AlsoGood& operator=(AlsoGood);
-};
-
-struct BadReturn {
- void operator=(const BadReturn&);
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should return 'BadReturn&' [misc-assign-operator-signature]
- const BadReturn& operator=(BadReturn&&);
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should return 'Bad
- void operator=(int);
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should return 'Bad
-};
-struct BadReturn2 {
- BadReturn2&& operator=(const BadReturn2&);
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should return 'Bad
- int operator=(BadReturn2&&);
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should return 'Bad
-};
-
-struct BadArgument {
- BadArgument& operator=(BadArgument&);
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should take 'BadArgument const&', 'BadArgument&&' or 'BadArgument'
- BadArgument& operator=(const BadArgument&&);
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should take 'BadAr
-};
-
-struct BadModifier {
- BadModifier& operator=(const BadModifier&) const;
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should not be marked 'const'
-};
-
-struct Deleted {
- // We don't check the return value of deleted operators.
- void operator=(const Deleted&) = delete;
- void operator=(Deleted&&) = delete;
-};
-
-class Private {
- // We don't check the return value of private operators.
- // 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: docs/clang-tidy/checks/misc-assign-operator.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/misc-assign-operator.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - misc-assign-operator
+
+misc-assign-operator
+====================
+
+
+Finds declarations of assign operators with the wrong return and/or argument
+types and definitions with good return type but wrong return statements.
+
+ * The return type must be ``Class&``.
+ * Works with move-assign and assign by value.
+ * Private and deleted operators are ignored.
+ * The operator must always return ``*this``
Index: docs/clang-tidy/checks/misc-assign-operator-signature.rst
===================================================================
--- docs/clang-tidy/checks/misc-assign-operator-signature.rst
+++ /dev/null
@@ -1,12 +0,0 @@
-.. title:: clang-tidy - misc-assign-operator-signature
-
-misc-assign-operator-signature
-==============================
-
-
-Finds declarations of assign operators with the wrong return and/or argument
-types.
-
- * The return type must be ``Class&``.
- * Works with move-assign and assign by value.
- * Private and deleted operators are ignored.
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -48,7 +48,7 @@
llvm-twine-local
misc-argument-comment
misc-assert-side-effect
- misc-assign-operator-signature
+ misc-assign-operator
misc-bool-pointer-implicit-conversion
misc-definitions-in-headers
misc-forward-declaration-namespace
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -12,7 +12,7 @@
#include "../ClangTidyModuleRegistry.h"
#include "ArgumentCommentCheck.h"
#include "AssertSideEffectCheck.h"
-#include "AssignOperatorSignatureCheck.h"
+#include "AssignOperatorCheck.h"
#include "BoolPointerImplicitConversionCheck.h"
#include "DefinitionsInHeadersCheck.h"
#include "ForwardDeclarationNamespaceCheck.h"
@@ -50,8 +50,8 @@
CheckFactories.registerCheck<ArgumentCommentCheck>("misc-argument-comment");
CheckFactories.registerCheck<AssertSideEffectCheck>(
"misc-assert-side-effect");
- CheckFactories.registerCheck<AssignOperatorSignatureCheck>(
- "misc-assign-operator-signature");
+ CheckFactories.registerCheck<AssignOperatorCheck>(
+ "misc-assign-operator");
CheckFactories.registerCheck<BoolPointerImplicitConversionCheck>(
"misc-bool-pointer-implicit-conversion");
CheckFactories.registerCheck<DefinitionsInHeadersCheck>(
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -3,7 +3,7 @@
add_clang_library(clangTidyMiscModule
ArgumentCommentCheck.cpp
AssertSideEffectCheck.cpp
- AssignOperatorSignatureCheck.cpp
+ AssignOperatorCheck.cpp
BoolPointerImplicitConversionCheck.cpp
DefinitionsInHeadersCheck.cpp
ForwardDeclarationNamespaceCheck.cpp
Index: clang-tidy/misc/AssignOperatorSignatureCheck.h
===================================================================
--- clang-tidy/misc/AssignOperatorSignatureCheck.h
+++ /dev/null
@@ -1,37 +0,0 @@
-//===--- AssignOperatorSignatureCheck.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_ASSIGNOPERATORSIGNATURECHECK_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_ASSIGNOPERATORSIGNATURECHECK_H
-
-#include "../ClangTidy.h"
-
-namespace clang {
-namespace tidy {
-namespace misc {
-
-/// Finds declarations of assign operators with the wrong return and/or argument
-/// types.
-///
-/// * The return type must be `Class&`.
-/// * Works with move-assign and assign by value.
-/// * Private and deleted operators are ignored.
-class AssignOperatorSignatureCheck : public ClangTidyCheck {
-public:
- AssignOperatorSignatureCheck(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_ASSIGNOPERATORSIGNATURECHECK_H
Index: clang-tidy/misc/AssignOperatorSignatureCheck.cpp
===================================================================
--- clang-tidy/misc/AssignOperatorSignatureCheck.cpp
+++ /dev/null
@@ -1,80 +0,0 @@
-//===--- AssignOperatorSignatureCheck.cpp - clang-tidy ----------*- C++ -*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#include "AssignOperatorSignatureCheck.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/ASTMatchers/ASTMatchers.h"
-
-using namespace clang::ast_matchers;
-
-namespace clang {
-namespace tidy {
-namespace misc {
-
-void AssignOperatorSignatureCheck::registerMatchers(
- ast_matchers::MatchFinder *Finder) {
- // Only register the matchers for C++; the functionality currently does not
- // provide any benefit to other languages, despite being benign.
- if (!getLangOpts().CPlusPlus)
- return;
-
- const auto HasGoodReturnType = cxxMethodDecl(returns(
- lValueReferenceType(pointee(unless(isConstQualified()),
- hasDeclaration(equalsBoundNode("class"))))));
-
- const auto IsSelf = qualType(
- anyOf(hasDeclaration(equalsBoundNode("class")),
- referenceType(pointee(hasDeclaration(equalsBoundNode("class"))))));
- const auto IsAssign =
- cxxMethodDecl(unless(anyOf(isDeleted(), isPrivate(), isImplicit())),
- hasName("operator="), ofClass(recordDecl().bind("class")))
- .bind("method");
- const auto IsSelfAssign =
- cxxMethodDecl(IsAssign, hasParameter(0, parmVarDecl(hasType(IsSelf))))
- .bind("method");
-
- Finder->addMatcher(
- cxxMethodDecl(IsAssign, unless(HasGoodReturnType)).bind("ReturnType"),
- this);
-
- const auto BadSelf = referenceType(
- anyOf(lValueReferenceType(pointee(unless(isConstQualified()))),
- rValueReferenceType(pointee(isConstQualified()))));
-
- Finder->addMatcher(
- cxxMethodDecl(IsSelfAssign,
- hasParameter(0, parmVarDecl(hasType(BadSelf))))
- .bind("ArgumentType"),
- 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");
- std::string Name = Method->getParent()->getName();
-
- static const char *const Messages[][2] = {
- {"ReturnType", "operator=() should return '%0&'"},
- {"ArgumentType", "operator=() should take '%0 const&', '%0&&' or '%0'"},
- {"cv", "operator=() should not be marked '%1'"}};
-
- for (const auto &Message : Messages) {
- if (Result.Nodes.getNodeAs<Decl>(Message[0]))
- diag(Method->getLocStart(), Message[1])
- << Name << (Method->isConst() ? "const" : "virtual");
- }
-}
-
-} // namespace misc
-} // namespace tidy
-} // namespace clang
Index: clang-tidy/misc/AssignOperatorCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/AssignOperatorCheck.h
@@ -0,0 +1,37 @@
+//===--- AssignOperatorCheck.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_ASSIGNOPERATORSIGNATURECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_ASSIGNOPERATORSIGNATURECHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Finds declarations of assignment operators with the wrong return and/or
+/// argument types.
+///
+/// * The return type must be `Class&`.
+/// * Works with move-assign and assign by value.
+/// * Private and deleted operators are ignored.
+class AssignOperatorCheck : public ClangTidyCheck {
+public:
+ AssignOperatorCheck(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_ASSIGNOPERATORSIGNATURECHECK_H
Index: clang-tidy/misc/AssignOperatorCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/AssignOperatorCheck.cpp
@@ -0,0 +1,92 @@
+//===--- AssignOperatorCheck.cpp - clang-tidy -------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "AssignOperatorCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void AssignOperatorCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
+ // Only register the matchers for C++; the functionality currently does not
+ // provide any benefit to other languages, despite being benign.
+ if (!getLangOpts().CPlusPlus)
+ return;
+
+ const auto HasGoodReturnType = cxxMethodDecl(returns(
+ lValueReferenceType(pointee(unless(isConstQualified()),
+ hasDeclaration(equalsBoundNode("class"))))));
+
+ const auto IsSelf = qualType(
+ anyOf(hasDeclaration(equalsBoundNode("class")),
+ referenceType(pointee(hasDeclaration(equalsBoundNode("class"))))));
+ const auto IsAssign =
+ cxxMethodDecl(unless(anyOf(isDeleted(), isPrivate(), isImplicit())),
+ hasName("operator="), ofClass(recordDecl().bind("class")))
+ .bind("method");
+ const auto IsSelfAssign =
+ cxxMethodDecl(IsAssign, hasParameter(0, parmVarDecl(hasType(IsSelf))))
+ .bind("method");
+
+ Finder->addMatcher(
+ cxxMethodDecl(IsAssign, unless(HasGoodReturnType)).bind("ReturnType"),
+ this);
+
+ const auto BadSelf = referenceType(
+ anyOf(lValueReferenceType(pointee(unless(isConstQualified()))),
+ rValueReferenceType(pointee(isConstQualified()))));
+
+ Finder->addMatcher(
+ cxxMethodDecl(IsSelfAssign,
+ hasParameter(0, parmVarDecl(hasType(BadSelf))))
+ .bind("ArgumentType"),
+ this);
+
+ Finder->addMatcher(
+ cxxMethodDecl(IsSelfAssign, anyOf(isConst(), isVirtual())).bind("cv"),
+ this);
+
+ const auto IsBadReturnStatement = returnStmt(unless(has(
+ unaryOperator(hasOperatorName("*"), hasUnaryOperand(cxxThisExpr())))));
+ const auto IsGoodAssign = cxxMethodDecl(IsAssign, HasGoodReturnType);
+
+ Finder->addMatcher(returnStmt(IsBadReturnStatement, hasAncestor(IsGoodAssign))
+ .bind("returnStmt"),
+ this);
+}
+
+void AssignOperatorCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Method = Result.Nodes.getNodeAs<CXXMethodDecl>("method");
+ const auto *RetStmt = Result.Nodes.getNodeAs<ReturnStmt>("returnStmt");
+ if (RetStmt) {
+ diag(RetStmt->getLocStart(), "operator=() should always return '*this'");
+ } else {
+ std::string Name = ;
+
+ static const char *const Messages[][2] = {
+ {"ReturnType", "operator=() should return '%0&'"},
+ {"ArgumentType", "operator=() should take '%0 const&', '%0&&' or '%0'"},
+ {"cv", "operator=() should not be marked '%1'"}};
+
+ for (const auto &Message : Messages) {
+ if (Result.Nodes.getNodeAs<Decl>(Message[0]))
+ diag(Method->getLocStart(), Message[1])
+ << Method->getParent()->getName()
+ << (Method->isConst() ? "const" : "virtual");
+ }
+ }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===================================================================
--- clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -10,7 +10,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
-#include "../misc/AssignOperatorSignatureCheck.h"
+#include "../misc/AssignOperatorCheck.h"
#include "ProBoundsArrayToPointerDecayCheck.h"
#include "ProBoundsConstantArrayIndexCheck.h"
#include "ProBoundsPointerArithmeticCheck.h"
@@ -50,7 +50,7 @@
"cppcoreguidelines-pro-type-union-access");
CheckFactories.registerCheck<ProTypeVarargCheck>(
"cppcoreguidelines-pro-type-vararg");
- CheckFactories.registerCheck<misc::AssignOperatorSignatureCheck>(
+ CheckFactories.registerCheck<misc::AssignOperatorCheck>(
"cppcoreguidelines-c-copy-assignment-signature");
}
};
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
