mirror of
https://github.com/capstone-engine/llvm-capstone.git
synced 2025-01-15 04:29:42 +00:00
[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 llvm-svn: 360540
This commit is contained in:
parent
ab8cde446b
commit
de7a30cb0a
@ -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>(
|
||||
|
@ -38,6 +38,7 @@ add_clang_library(clangTidyBugproneModule
|
||||
TooSmallLoopVariableCheck.cpp
|
||||
UndefinedMemoryManipulationCheck.cpp
|
||||
UndelegatedConstructorCheck.cpp
|
||||
UnhandledSelfAssignmentCheck.cpp
|
||||
UnusedRaiiCheck.cpp
|
||||
UnusedReturnValueCheck.cpp
|
||||
UseAfterMoveCheck.cpp
|
||||
|
@ -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
|
@ -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
|
@ -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.
|
||||
|
@ -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;
|
||||
}
|
||||
};
|
@ -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
|
||||
|
@ -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;
|
||||
};
|
Loading…
x
Reference in New Issue
Block a user