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( "cppcoreguidelines-pro-type-vararg"); - CheckFactories.registerCheck( + CheckFactories.registerCheck( "cppcoreguidelines-c-copy-assignment-signature"); } }; 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 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 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,91 @@ +//===--- 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("method"); + const auto *RetStmt = Result.Nodes.getNodeAs("returnStmt"); + if (RetStmt) { + diag(RetStmt->getLocStart(), "operator=() should always return '*this'"); + } else { + 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(Message[0])) + diag(Method->getLocStart(), Message[1]) + << Name << (Method->isConst() ? "const" : "virtual"); + } + } +} + +} // namespace misc +} // namespace tidy +} // namespace clang 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("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(Message[0])) - diag(Method->getLocStart(), Message[1]) - << Name << (Method->isConst() ? "const" : "virtual"); - } -} - -} // namespace misc -} // namespace tidy -} // namespace clang 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/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("misc-argument-comment"); CheckFactories.registerCheck( "misc-assert-side-effect"); - CheckFactories.registerCheck( - "misc-assign-operator-signature"); + CheckFactories.registerCheck( + "misc-assign-operator"); CheckFactories.registerCheck( "misc-bool-pointer-implicit-conversion"); CheckFactories.registerCheck( 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: 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/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: 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: 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 + +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; + } +};