[clang-tools-extra] r360540 - [clang-tidy] new check: bugprone-unhandled-self-assignment
Tamas Zolnai via cfe-commits
cfe-commits at lists.llvm.org
Sun May 12 05:23:56 PDT 2019
Author: ztamas
Date: Sun May 12 05:23:56 2019
New Revision: 360540
URL: http://llvm.org/viewvc/llvm-project?rev=360540&view=rev
Log:
[clang-tidy] new check: bugprone-unhandled-self-assignment
Summary:
This check searches for copy assignment operators which might not handle self-assignment properly. There are three patterns of
handling a self assignment situation: self check, copy-and-swap or the less common copy-and-move. The new check warns if none of
these patterns is found in a user defined implementation.
See also:
OOP54-CPP. Gracefully handle self-copy assignment
https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP54-CPP.+Gracefully+handle+self-copy+assignment
Reviewers: JonasToth, alexfh, hokein, aaron.ballman
Subscribers: riccibruno, Eugene.Zelenko, mgorny, xazax.hun, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D60507
Added:
clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
clang-tools-extra/trunk/test/clang-tidy/bugprone-unhandled-self-assignment.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified: clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp?rev=360540&r1=360539&r2=360540&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp Sun May 12 05:23:56 2019
@@ -46,6 +46,7 @@
#include "TooSmallLoopVariableCheck.h"
#include "UndefinedMemoryManipulationCheck.h"
#include "UndelegatedConstructorCheck.h"
+#include "UnhandledSelfAssignmentCheck.h"
#include "UnusedRaiiCheck.h"
#include "UnusedReturnValueCheck.h"
#include "UseAfterMoveCheck.h"
@@ -132,6 +133,8 @@ public:
"bugprone-undefined-memory-manipulation");
CheckFactories.registerCheck<UndelegatedConstructorCheck>(
"bugprone-undelegated-constructor");
+ CheckFactories.registerCheck<UnhandledSelfAssignmentCheck>(
+ "bugprone-unhandled-self-assignment");
CheckFactories.registerCheck<UnusedRaiiCheck>(
"bugprone-unused-raii");
CheckFactories.registerCheck<UnusedReturnValueCheck>(
Modified: clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt?rev=360540&r1=360539&r2=360540&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt Sun May 12 05:23:56 2019
@@ -38,6 +38,7 @@ add_clang_library(clangTidyBugproneModul
TooSmallLoopVariableCheck.cpp
UndefinedMemoryManipulationCheck.cpp
UndelegatedConstructorCheck.cpp
+ UnhandledSelfAssignmentCheck.cpp
UnusedRaiiCheck.cpp
UnusedReturnValueCheck.cpp
UseAfterMoveCheck.cpp
Added: clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp?rev=360540&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp Sun May 12 05:23:56 2019
@@ -0,0 +1,99 @@
+//===--- UnhandledSelfAssignmentCheck.cpp - clang-tidy --------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "UnhandledSelfAssignmentCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+void UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus)
+ return;
+
+ // We don't care about deleted, default or implicit operator implementations.
+ const auto IsUserDefined = cxxMethodDecl(
+ isDefinition(), unless(anyOf(isDeleted(), isImplicit(), isDefaulted())));
+
+ // We don't need to worry when a copy assignment operator gets the other
+ // object by value.
+ const auto HasReferenceParam =
+ cxxMethodDecl(hasParameter(0, parmVarDecl(hasType(referenceType()))));
+
+ // Self-check: Code compares something with 'this' pointer. We don't check
+ // whether it is actually the parameter what we compare.
+ const auto HasNoSelfCheck = cxxMethodDecl(unless(hasDescendant(
+ binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
+ has(ignoringParenCasts(cxxThisExpr()))))));
+
+ // Both copy-and-swap and copy-and-move method creates a copy first and
+ // assign it to 'this' with swap or move.
+ // In the non-template case, we can search for the copy constructor call.
+ const auto HasNonTemplateSelfCopy = cxxMethodDecl(
+ ofClass(cxxRecordDecl(unless(hasAncestor(classTemplateDecl())))),
+ hasDescendant(cxxConstructExpr(hasDeclaration(cxxConstructorDecl(
+ isCopyConstructor(), ofClass(equalsBoundNode("class")))))));
+
+ // In the template case, we need to handle two separate cases: 1) a local
+ // variable is created with the copy, 2) copy is created only as a temporary
+ // object.
+ const auto HasTemplateSelfCopy = cxxMethodDecl(
+ ofClass(cxxRecordDecl(hasAncestor(classTemplateDecl()))),
+ anyOf(hasDescendant(
+ varDecl(hasType(cxxRecordDecl(equalsBoundNode("class"))),
+ hasDescendant(parenListExpr()))),
+ hasDescendant(cxxUnresolvedConstructExpr(hasDescendant(declRefExpr(
+ hasType(cxxRecordDecl(equalsBoundNode("class")))))))));
+
+ // If inside the copy assignment operator another assignment operator is
+ // called on 'this' we assume that self-check might be handled inside
+ // this nested operator.
+ const auto HasNoNestedSelfAssign =
+ cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
+ hasName("operator="), ofClass(equalsBoundNode("class"))))))));
+
+ // Matcher for standard smart pointers.
+ const auto SmartPointerType = qualType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(classTemplateSpecializationDecl(
+ hasAnyName("::std::shared_ptr", "::std::unique_ptr",
+ "::std::weak_ptr", "::std::auto_ptr"),
+ templateArgumentCountIs(1))))));
+
+ // We will warn only if the class has a pointer or a C array field which
+ // probably causes a problem during self-assignment (e.g. first resetting the
+ // pointer member, then trying to access the object pointed by the pointer, or
+ // memcpy overlapping arrays).
+ const auto ThisHasSuspiciousField = cxxMethodDecl(ofClass(cxxRecordDecl(
+ has(fieldDecl(anyOf(hasType(pointerType()), hasType(SmartPointerType),
+ hasType(arrayType())))))));
+
+ Finder->addMatcher(
+ cxxMethodDecl(ofClass(cxxRecordDecl().bind("class")),
+ isCopyAssignmentOperator(), IsUserDefined,
+ HasReferenceParam, HasNoSelfCheck,
+ unless(HasNonTemplateSelfCopy), unless(HasTemplateSelfCopy),
+ HasNoNestedSelfAssign, ThisHasSuspiciousField)
+ .bind("copyAssignmentOperator"),
+ this);
+}
+
+void UnhandledSelfAssignmentCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *MatchedDecl =
+ Result.Nodes.getNodeAs<CXXMethodDecl>("copyAssignmentOperator");
+ diag(MatchedDecl->getLocation(),
+ "operator=() does not handle self-assignment properly");
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
Added: clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h?rev=360540&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h Sun May 12 05:23:56 2019
@@ -0,0 +1,36 @@
+//===--- UnhandledSelfAssignmentCheck.h - clang-tidy ------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNHANDLEDSELFASSIGNMENTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNHANDLEDSELFASSIGNMENTCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Finds user-defined copy assignment operators which do not protect the code
+/// against self-assignment either by checking self-assignment explicitly or
+/// using the copy-and-swap or the copy-and-move method.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-unhandled-self-assignment.html
+class UnhandledSelfAssignmentCheck : public ClangTidyCheck {
+public:
+ UnhandledSelfAssignmentCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNHANDLEDSELFASSIGNMENTCHECK_H
Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=360540&r1=360539&r2=360540&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Sun May 12 05:23:56 2019
@@ -101,6 +101,13 @@ Improvements to clang-tidy
Finds and fixes ``absl::Time`` subtraction expressions to do subtraction
in the Time domain instead of the numeric domain.
+- New :doc:`bugprone-unhandled-self-assignment
+ <clang-tidy/checks/bugprone-unhandled-self-assignment>` check.
+
+ Finds user-defined copy assignment operators which do not protect the code
+ against self-assignment either by checking self-assignment explicitly or
+ using the copy-and-swap or the copy-and-move method.
+
- New :doc:`google-readability-avoid-underscore-in-googletest-name
<clang-tidy/checks/google-readability-avoid-underscore-in-googletest-name>`
check.
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst?rev=360540&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst Sun May 12 05:23:56 2019
@@ -0,0 +1,116 @@
+.. title:: clang-tidy - bugprone-unhandled-self-assignment
+
+bugprone-unhandled-self-assignment
+==================================
+
+Finds user-defined copy assignment operators which do not protect the code
+against self-assignment either by checking self-assignment explicitly or
+using the copy-and-swap or the copy-and-move method.
+
+This check now searches only those classes which have any pointer or C array field
+to avoid false positives. In case of a pointer or a C array, it's likely that self-copy
+assignment breaks the object if the copy assignment operator was not written with care.
+
+See also:
+`OOP54-CPP. Gracefully handle self-copy assignment
+<https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP54-CPP.+Gracefully+handle+self-copy+assignment>`_
+
+A copy assignment operator must prevent that self-copy assignment ruins the
+object state. A typical use case is when the class has a pointer field
+and the copy assignment operator first releases the pointed object and
+then tries to assign it:
+
+.. code-block:: c++
+
+ class T {
+ int* p;
+
+ public:
+ T(const T &rhs) : p(rhs.p ? new int(*rhs.p) : nullptr) {}
+ ~T() { delete p; }
+
+ // ...
+
+ T& operator=(const T &rhs) {
+ delete p;
+ p = new int(*rhs.p);
+ return *this;
+ }
+ };
+
+There are two common C++ patterns to avoid this problem. The first is
+the self-assignment check:
+
+.. code-block:: c++
+
+ class T {
+ int* p;
+
+ public:
+ T(const T &rhs) : p(rhs.p ? new int(*rhs.p) : nullptr) {}
+ ~T() { delete p; }
+
+ // ...
+
+ T& operator=(const T &rhs) {
+ if(this == &rhs)
+ return *this;
+
+ delete p;
+ p = new int(*rhs.p);
+ return *this;
+ }
+ };
+
+The second one is the copy-and-swap method when we create a temporary copy
+(using the copy constructor) and then swap this temporary object with ``this``:
+
+.. code-block:: c++
+
+ class T {
+ int* p;
+
+ public:
+ T(const T &rhs) : p(rhs.p ? new int(*rhs.p) : nullptr) {}
+ ~T() { delete p; }
+
+ // ...
+
+ void swap(T &rhs) {
+ using std::swap;
+ swap(p, rhs.p);
+ }
+
+ T& operator=(const T &rhs) {
+ T(rhs).swap(*this);
+ return *this;
+ }
+ };
+
+There is a third pattern which is less common. Let's call it the copy-and-move method
+when we create a temporary copy (using the copy constructor) and then move this
+temporary object into ``this`` (needs a move assignment operator):
+
+.. code-block:: c++
+
+ class T {
+ int* p;
+
+ public:
+ T(const T &rhs) : p(rhs.p ? new int(*rhs.p) : nullptr) {}
+ ~T() { delete p; }
+
+ // ...
+
+ T& operator=(const T &rhs) {
+ T t = rhs;
+ *this = std::move(t);
+ return *this;
+ }
+
+ T& operator=(T &&rhs) {
+ p = rhs.p;
+ rhs.p = nullptr;
+ return *this;
+ }
+ };
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=360540&r1=360539&r2=360540&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Sun May 12 05:23:56 2019
@@ -71,6 +71,7 @@ Clang-Tidy Checks
bugprone-too-small-loop-variable
bugprone-undefined-memory-manipulation
bugprone-undelegated-constructor
+ bugprone-unhandled-self-assignment
bugprone-unused-raii
bugprone-unused-return-value
bugprone-use-after-move
Added: clang-tools-extra/trunk/test/clang-tidy/bugprone-unhandled-self-assignment.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-unhandled-self-assignment.cpp?rev=360540&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-unhandled-self-assignment.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-unhandled-self-assignment.cpp Sun May 12 05:23:56 2019
@@ -0,0 +1,579 @@
+// RUN: %check_clang_tidy %s bugprone-unhandled-self-assignment %t
+
+namespace std {
+
+template <class T>
+void swap(T x, T y) {
+}
+
+template <class T>
+T &&move(T x) {
+}
+
+template <class T>
+class unique_ptr {
+};
+
+template <class T>
+class shared_ptr {
+};
+
+template <class T>
+class weak_ptr {
+};
+
+template <class T>
+class auto_ptr {
+};
+
+} // namespace std
+
+void assert(int expression){};
+
+///////////////////////////////////////////////////////////////////
+/// Test cases correctly caught by the check.
+
+class PtrField {
+public:
+ PtrField &operator=(const PtrField &object);
+
+private:
+ int *p;
+};
+
+PtrField &PtrField::operator=(const PtrField &object) {
+ // CHECK-MESSAGES: [[@LINE-1]]:21: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+ // ...
+ return *this;
+}
+
+// Class with an inline operator definition.
+class InlineDefinition {
+public:
+ InlineDefinition &operator=(const InlineDefinition &object) {
+ // CHECK-MESSAGES: [[@LINE-1]]:21: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+ // ...
+ return *this;
+ }
+
+private:
+ int *p;
+};
+
+class UniquePtrField {
+public:
+ UniquePtrField &operator=(const UniquePtrField &object) {
+ // CHECK-MESSAGES: [[@LINE-1]]:19: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+ // ...
+ return *this;
+ }
+
+private:
+ std::unique_ptr<int> p;
+};
+
+class SharedPtrField {
+public:
+ SharedPtrField &operator=(const SharedPtrField &object) {
+ // CHECK-MESSAGES: [[@LINE-1]]:19: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+ // ...
+ return *this;
+ }
+
+private:
+ std::shared_ptr<int> p;
+};
+
+class WeakPtrField {
+public:
+ WeakPtrField &operator=(const WeakPtrField &object) {
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+ // ...
+ return *this;
+ }
+
+private:
+ std::weak_ptr<int> p;
+};
+
+class AutoPtrField {
+public:
+ AutoPtrField &operator=(const AutoPtrField &object) {
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+ // ...
+ return *this;
+ }
+
+private:
+ std::auto_ptr<int> p;
+};
+
+// Class with C array field.
+class CArrayField {
+public:
+ CArrayField &operator=(const CArrayField &object) {
+ // CHECK-MESSAGES: [[@LINE-1]]:16: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+ // ...
+ return *this;
+ }
+
+private:
+ int array[256];
+};
+
+// Make sure to not ignore cases when the operator definition calls
+// a copy constructor of another class.
+class CopyConstruct {
+public:
+ CopyConstruct &operator=(const CopyConstruct &object) {
+ // CHECK-MESSAGES: [[@LINE-1]]:18: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+ WeakPtrField a;
+ WeakPtrField b(a);
+ // ...
+ return *this;
+ }
+
+private:
+ int *p;
+};
+
+// Make sure to not ignore cases when the operator definition calls
+// a copy assignment operator of another class.
+class AssignOperator {
+public:
+ AssignOperator &operator=(const AssignOperator &object) {
+ // CHECK-MESSAGES: [[@LINE-1]]:19: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+ a.operator=(object.a);
+ // ...
+ return *this;
+ }
+
+private:
+ int *p;
+ WeakPtrField a;
+};
+
+class NotSelfCheck {
+public:
+ NotSelfCheck &operator=(const NotSelfCheck &object) {
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+ if (&object == this->doSomething()) {
+ // ...
+ }
+ return *this;
+ }
+
+ void *doSomething() {
+ return p;
+ }
+
+private:
+ int *p;
+};
+
+template <class T>
+class TemplatePtrField {
+public:
+ TemplatePtrField<T> &operator=(const TemplatePtrField<T> &object) {
+ // CHECK-MESSAGES: [[@LINE-1]]:24: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+ // ...
+ return *this;
+ }
+
+private:
+ T *p;
+};
+
+template <class T>
+class TemplateCArrayField {
+public:
+ TemplateCArrayField<T> &operator=(const TemplateCArrayField<T> &object) {
+ // CHECK-MESSAGES: [[@LINE-1]]:27: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+ // ...
+ return *this;
+ }
+
+private:
+ T p[256];
+};
+
+// Other template class's constructor is called inside a declaration.
+template <class T>
+class WrongTemplateCopyAndMove {
+public:
+ WrongTemplateCopyAndMove<T> &operator=(const WrongTemplateCopyAndMove<T> &object) {
+ // CHECK-MESSAGES: [[@LINE-1]]:32: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+ TemplatePtrField<T> temp;
+ TemplatePtrField<T> temp2(temp);
+ return *this;
+ }
+
+private:
+ T *p;
+};
+
+///////////////////////////////////////////////////////////////////
+/// Test cases correctly ignored by the check.
+
+// Self-assignment is checked using the equality operator.
+class SelfCheck1 {
+public:
+ SelfCheck1 &operator=(const SelfCheck1 &object) {
+ if (this == &object)
+ return *this;
+ // ...
+ return *this;
+ }
+
+private:
+ int *p;
+};
+
+class SelfCheck2 {
+public:
+ SelfCheck2 &operator=(const SelfCheck2 &object) {
+ if (&object == this)
+ return *this;
+ // ...
+ return *this;
+ }
+
+private:
+ int *p;
+};
+
+// Self-assignment is checked using the inequality operator.
+class SelfCheck3 {
+public:
+ SelfCheck3 &operator=(const SelfCheck3 &object) {
+ if (this != &object) {
+ // ...
+ }
+ return *this;
+ }
+
+private:
+ int *p;
+};
+
+class SelfCheck4 {
+public:
+ SelfCheck4 &operator=(const SelfCheck4 &object) {
+ if (&object != this) {
+ // ...
+ }
+ return *this;
+ }
+
+private:
+ int *p;
+};
+
+template <class T>
+class TemplateSelfCheck {
+public:
+ TemplateSelfCheck<T> &operator=(const TemplateSelfCheck<T> &object) {
+ if (&object != this) {
+ // ...
+ }
+ return *this;
+ }
+
+private:
+ T *p;
+};
+
+// There is no warning if the copy assignment operator gets the object by value.
+class PassedByValue {
+public:
+ PassedByValue &operator=(PassedByValue object) {
+ // ...
+ return *this;
+ }
+
+private:
+ int *p;
+};
+
+// User-defined swap method calling std::swap inside.
+class CopyAndSwap1 {
+public:
+ CopyAndSwap1 &operator=(const CopyAndSwap1 &object) {
+ CopyAndSwap1 temp(object);
+ doSwap(temp);
+ return *this;
+ }
+
+private:
+ int *p;
+
+ void doSwap(CopyAndSwap1 &object) {
+ using std::swap;
+ swap(p, object.p);
+ }
+};
+
+// User-defined swap method used with passed-by-value parameter.
+class CopyAndSwap2 {
+public:
+ CopyAndSwap2 &operator=(CopyAndSwap2 object) {
+ doSwap(object);
+ return *this;
+ }
+
+private:
+ int *p;
+
+ void doSwap(CopyAndSwap2 &object) {
+ using std::swap;
+ swap(p, object.p);
+ }
+};
+
+// Copy-and-swap method is used but without creating a separate method for it.
+class CopyAndSwap3 {
+public:
+ CopyAndSwap3 &operator=(const CopyAndSwap3 &object) {
+ CopyAndSwap3 temp(object);
+ std::swap(p, temp.p);
+ return *this;
+ }
+
+private:
+ int *p;
+};
+
+template <class T>
+class TemplateCopyAndSwap {
+public:
+ TemplateCopyAndSwap<T> &operator=(const TemplateCopyAndSwap<T> &object) {
+ TemplateCopyAndSwap<T> temp(object);
+ std::swap(p, temp.p);
+ return *this;
+ }
+
+private:
+ T *p;
+};
+
+// Move semantics is used on a temporary copy of the object.
+class CopyAndMove1 {
+public:
+ CopyAndMove1 &operator=(const CopyAndMove1 &object) {
+ CopyAndMove1 temp(object);
+ *this = std::move(temp);
+ return *this;
+ }
+
+private:
+ int *p;
+};
+
+// There is no local variable for the temporary copy.
+class CopyAndMove2 {
+public:
+ CopyAndMove2 &operator=(const CopyAndMove2 &object) {
+ *this = std::move(CopyAndMove2(object));
+ return *this;
+ }
+
+private:
+ int *p;
+};
+
+template <class T>
+class TemplateCopyAndMove {
+public:
+ TemplateCopyAndMove<T> &operator=(const TemplateCopyAndMove<T> &object) {
+ TemplateCopyAndMove<T> temp(object);
+ *this = std::move(temp);
+ return *this;
+ }
+
+private:
+ T *p;
+};
+
+// There is no local variable for the temporary copy.
+template <class T>
+class TemplateCopyAndMove2 {
+public:
+ TemplateCopyAndMove2<T> &operator=(const TemplateCopyAndMove2<T> &object) {
+ *this = std::move(TemplateCopyAndMove2<T>(object));
+ return *this;
+ }
+
+private:
+ T *p;
+};
+
+// We should not catch move assignment operators.
+class MoveAssignOperator {
+public:
+ MoveAssignOperator &operator=(MoveAssignOperator &&object) {
+ // ...
+ return *this;
+ }
+
+private:
+ int *p;
+};
+
+// We ignore copy assignment operators without user-defined implementation.
+class DefaultOperator {
+public:
+ DefaultOperator &operator=(const DefaultOperator &object) = default;
+
+private:
+ int *p;
+};
+
+class DeletedOperator {
+public:
+ DeletedOperator &operator=(const DefaultOperator &object) = delete;
+
+private:
+ int *p;
+};
+
+class ImplicitOperator {
+private:
+ int *p;
+};
+
+// Check ignores those classes which has no any pointer or array field.
+class TrivialFields {
+public:
+ TrivialFields &operator=(const TrivialFields &object) {
+ // ...
+ return *this;
+ }
+
+private:
+ int m;
+ float f;
+ double d;
+ bool b;
+};
+
+// There is no warning when the class calls another assignment operator on 'this'
+// inside the copy assignment operator's definition.
+class AssignIsForwarded {
+public:
+ AssignIsForwarded &operator=(const AssignIsForwarded &object) {
+ operator=(object.p);
+ return *this;
+ }
+
+ AssignIsForwarded &operator=(int *pp) {
+ if (p != pp) {
+ delete p;
+ p = new int(*pp);
+ }
+ return *this;
+ }
+
+private:
+ int *p;
+};
+
+// Assertion is a valid way to say that self-assignment is not expected to happen.
+class AssertGuard {
+public:
+ AssertGuard &operator=(const AssertGuard &object) {
+ assert(this != &object);
+ // ...
+ return *this;
+ }
+
+private:
+ int *p;
+};
+
+// Make sure we don't catch this operator=() as a copy assignment operator.
+// Note that RHS has swapped template arguments.
+template <typename Ty, typename Uy>
+class NotACopyAssignmentOperator {
+ Ty *Ptr1;
+ Uy *Ptr2;
+
+public:
+ NotACopyAssignmentOperator& operator=(const NotACopyAssignmentOperator<Uy, Ty> &RHS) {
+ Ptr1 = RHS.getUy();
+ Ptr2 = RHS.getTy();
+ return *this;
+ }
+
+ Ty *getTy() const { return Ptr1; }
+ Uy *getUy() const { return Ptr2; }
+};
+
+///////////////////////////////////////////////////////////////////
+/// Test cases which should be caught by the check.
+
+// TODO: handle custom pointers.
+template <class T>
+class custom_ptr {
+};
+
+class CustomPtrField {
+public:
+ CustomPtrField &operator=(const CustomPtrField &object) {
+ // ...
+ return *this;
+ }
+
+private:
+ custom_ptr<int> p;
+};
+
+/////////////////////////////////////////////////////////////////////////////////////////////////////
+/// False positives: These are self-assignment safe, but they don't use any of the three patterns.
+
+class ArrayCopy {
+public:
+ ArrayCopy &operator=(const ArrayCopy &object) {
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+ for (int i = 0; i < 256; i++)
+ array[i] = object.array[i];
+ return *this;
+ }
+
+private:
+ int array[256];
+};
+
+class GetterSetter {
+public:
+ GetterSetter &operator=(const GetterSetter &object) {
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+ setValue(object.getValue());
+ return *this;
+ }
+
+ int *getValue() const { return value; }
+
+ void setValue(int *newPtr) {
+ int *pTmp(newPtr ? new int(*newPtr) : nullptr);
+ std::swap(value, pTmp);
+ delete pTmp;
+ }
+
+private:
+ int *value;
+};
+
+class CustomSelfCheck {
+public:
+ CustomSelfCheck &operator=(const CustomSelfCheck &object) {
+ // CHECK-MESSAGES: [[@LINE-1]]:20: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+ if (index != object.index) {
+ // ...
+ }
+ return *this;
+ }
+
+private:
+ int *value;
+ int index;
+};
More information about the cfe-commits
mailing list