This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE361550: [clang-tidy]: Add cert-oop54-cpp alias for
bugprone-unhandled-self-assignment (authored by ztamas, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D62192?vs=200775&id=201051#toc
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62192/new/
https://reviews.llvm.org/D62192
Files:
clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h
clang-tidy/cert/CERTTidyModule.cpp
clang-tidy/cert/CMakeLists.txt
docs/ReleaseNotes.rst
docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
docs/clang-tidy/checks/cert-oop54-cpp.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/bugprone-unhandled-self-assignment-warn-only-if-this-has-suspicious-field.cpp
test/clang-tidy/cert-oop54-cpp.cpp
Index: clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
===================================================================
--- clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -16,6 +16,18 @@
namespace tidy {
namespace bugprone {
+UnhandledSelfAssignmentCheck::UnhandledSelfAssignmentCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ WarnOnlyIfThisHasSuspiciousField(
+ Options.get("WarnOnlyIfThisHasSuspiciousField", true)) {}
+
+void UnhandledSelfAssignmentCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "WarnOnlyIfThisHasSuspiciousField",
+ WarnOnlyIfThisHasSuspiciousField);
+}
+
void UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
if (!getLangOpts().CPlusPlus)
return;
@@ -61,29 +73,32 @@
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);
+ DeclarationMatcher AdditionalMatcher = cxxMethodDecl();
+ if (WarnOnlyIfThisHasSuspiciousField) {
+ // 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).
+ AdditionalMatcher = 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, AdditionalMatcher)
+ .bind("copyAssignmentOperator"),
+ this);
}
void UnhandledSelfAssignmentCheck::check(
Index: clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h
===================================================================
--- clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h
+++ clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h
@@ -23,10 +23,14 @@
/// 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) {}
+ UnhandledSelfAssignmentCheck(StringRef Name, ClangTidyContext *Context);
+
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ const bool WarnOnlyIfThisHasSuspiciousField;
};
} // namespace bugprone
Index: clang-tidy/cert/CERTTidyModule.cpp
===================================================================
--- clang-tidy/cert/CERTTidyModule.cpp
+++ clang-tidy/cert/CERTTidyModule.cpp
@@ -9,6 +9,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "../bugprone/UnhandledSelfAssignmentCheck.h"
#include "../google/UnnamedNamespaceInHeaderCheck.h"
#include "../misc/NewDeleteOverloadsCheck.h"
#include "../misc/NonCopyableObjects.h"
@@ -49,6 +50,8 @@
// OOP
CheckFactories.registerCheck<performance::MoveConstructorInitCheck>(
"cert-oop11-cpp");
+ CheckFactories.registerCheck<bugprone::UnhandledSelfAssignmentCheck>(
+ "cert-oop54-cpp");
// ERR
CheckFactories.registerCheck<misc::ThrowByValueCatchByReferenceCheck>(
"cert-err09-cpp");
@@ -85,6 +88,7 @@
ClangTidyOptions Options;
ClangTidyOptions::OptionMap &Opts = Options.CheckOptions;
Opts["cert-dcl16-c.NewSuffixes"] = "L;LL;LU;LLU";
+ Opts["cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField"] = "0";
return Options;
}
};
Index: clang-tidy/cert/CMakeLists.txt
===================================================================
--- clang-tidy/cert/CMakeLists.txt
+++ clang-tidy/cert/CMakeLists.txt
@@ -20,6 +20,7 @@
clangBasic
clangLex
clangTidy
+ clangTidyBugproneModule
clangTidyGoogleModule
clangTidyMiscModule
clangTidyPerformanceModule
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -134,6 +134,11 @@
subclasses of ``NSObject`` and recommends calling a superclass initializer
instead.
+- New alias :doc:`cert-oop54-cpp
+ <clang-tidy/checks/cert-oop54-cpp>` to
+ :doc:`bugprone-unhandled-self-assignment
+ <clang-tidy/checks/bugprone-unhandled-self-assignment>` was added.
+
- New alias :doc:`cppcoreguidelines-explicit-virtual-functions
<clang-tidy/checks/cppcoreguidelines-explicit-virtual-functions>` to
:doc:`modernize-use-override
Index: docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
===================================================================
--- docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
+++ docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
@@ -3,11 +3,14 @@
bugprone-unhandled-self-assignment
==================================
+`cert-oop54-cpp` redirects here as an alias for this check. For the CERT alias,
+the `WarnOnlyIfThisHasSuspiciousField` option is set to `0`.
+
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
+By default, this check 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.
@@ -114,3 +117,8 @@
return *this;
}
};
+
+.. option:: WarnOnlyIfThisHasSuspiciousField
+
+ When non-zero, the check will warn only if the container class of the copy assignment operator
+ has any suspicious fields (pointer or C array). This option is set to `1` by default.
Index: docs/clang-tidy/checks/cert-oop54-cpp.rst
===================================================================
--- docs/clang-tidy/checks/cert-oop54-cpp.rst
+++ docs/clang-tidy/checks/cert-oop54-cpp.rst
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - cert-oop54-cpp
+.. meta::
+ :http-equiv=refresh: 5;URL=bugprone-unhandled-self-assignment.html
+
+cert-oop54-cpp
+==============
+
+The cert-oop54-cpp check is an alias, please see
+`bugprone-unhandled-self-assignment <bugprone-unhandled-self-assignment.html>`_
+for more information.
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -98,6 +98,7 @@
cert-msc50-cpp
cert-msc51-cpp
cert-oop11-cpp (redirects to performance-move-constructor-init) <cert-oop11-cpp>
+ cert-oop54-cpp (redirects to bugprone-unhandled-self-assignment) <cert-oop54-cpp>
cppcoreguidelines-avoid-c-arrays (redirects to modernize-avoid-c-arrays) <cppcoreguidelines-avoid-c-arrays>
cppcoreguidelines-avoid-goto
cppcoreguidelines-avoid-magic-numbers (redirects to readability-magic-numbers) <cppcoreguidelines-avoid-magic-numbers>
Index: test/clang-tidy/bugprone-unhandled-self-assignment-warn-only-if-this-has-suspicious-field.cpp
===================================================================
--- test/clang-tidy/bugprone-unhandled-self-assignment-warn-only-if-this-has-suspicious-field.cpp
+++ test/clang-tidy/bugprone-unhandled-self-assignment-warn-only-if-this-has-suspicious-field.cpp
@@ -0,0 +1,41 @@
+// RUN: %check_clang_tidy %s bugprone-unhandled-self-assignment %t -- \
+// RUN: -config="{CheckOptions: \
+// RUN: [{key: bugprone-unhandled-self-assignment.WarnOnlyIfThisHasSuspiciousField, \
+// RUN: value: 0}]}"
+
+// Classes with pointer field are still caught.
+class PtrField {
+public:
+ PtrField &operator=(const PtrField &object) {
+ // CHECK-MESSAGES: [[@LINE-1]]:13: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+ return *this;
+ }
+
+private:
+ int *p;
+};
+
+// With the option, check catches classes with trivial fields.
+class TrivialFields {
+public:
+ TrivialFields &operator=(const TrivialFields &object) {
+ // CHECK-MESSAGES: [[@LINE-1]]:18: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+ return *this;
+ }
+
+private:
+ int m;
+ float f;
+ double d;
+ bool b;
+};
+
+// The check warns also when there is no field at all.
+// In this case, user-defined copy assignment operator is useless anyway.
+class ClassWithoutFields {
+public:
+ ClassWithoutFields &operator=(const ClassWithoutFields &object) {
+ // CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+ return *this;
+ }
+};
Index: test/clang-tidy/cert-oop54-cpp.cpp
===================================================================
--- test/clang-tidy/cert-oop54-cpp.cpp
+++ test/clang-tidy/cert-oop54-cpp.cpp
@@ -0,0 +1,16 @@
+// RUN: %check_clang_tidy %s cert-oop54-cpp %t
+
+// Test whether bugprone-unhandled-self-assignment.WarnOnlyIfThisHasSuspiciousField option is set correctly.
+class TrivialFields {
+public:
+ TrivialFields &operator=(const TrivialFields &object) {
+ // CHECK-MESSAGES: [[@LINE-1]]:18: warning: operator=() does not handle self-assignment properly [cert-oop54-cpp]
+ return *this;
+ }
+
+private:
+ int m;
+ float f;
+ double d;
+ bool b;
+};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
