[clang-tools-extra] r361550 - [clang-tidy]: Add cert-oop54-cpp alias for bugprone-unhandled-self-assignment
Tamas Zolnai via cfe-commits
cfe-commits at lists.llvm.org
Thu May 23 13:29:04 PDT 2019
Author: ztamas
Date: Thu May 23 13:29:04 2019
New Revision: 361550
URL: http://llvm.org/viewvc/llvm-project?rev=361550&view=rev
Log:
[clang-tidy]: Add cert-oop54-cpp alias for bugprone-unhandled-self-assignment
Summary:
Added WarnOnlyIfThisHasSuspiciousField option to allow
to catch any copy assignment operator independently from
the container class's fields.
Added the cert alias using this option.
Reviewers: aaron.ballman
Reviewed By: aaron.ballman
Subscribers: mgorny, Eugene.Zelenko, xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D62192
Added:
clang-tools-extra/trunk/docs/clang-tidy/checks/cert-oop54-cpp.rst
clang-tools-extra/trunk/test/clang-tidy/bugprone-unhandled-self-assignment-warn-only-if-this-has-suspicious-field.cpp
clang-tools-extra/trunk/test/clang-tidy/cert-oop54-cpp.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h
clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified: 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=361550&r1=361549&r2=361550&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp Thu May 23 13:29:04 2019
@@ -16,6 +16,18 @@ namespace clang {
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 @@ void UnhandledSelfAssignmentCheck::regis
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(
Modified: 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=361550&r1=361549&r2=361550&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h Thu May 23 13:29:04 2019
@@ -23,10 +23,14 @@ namespace bugprone {
/// 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
Modified: clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp?rev=361550&r1=361549&r2=361550&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp Thu May 23 13:29:04 2019
@@ -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 @@ public:
// 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 @@ public:
ClangTidyOptions Options;
ClangTidyOptions::OptionMap &Opts = Options.CheckOptions;
Opts["cert-dcl16-c.NewSuffixes"] = "L;LL;LU;LLU";
+ Opts["cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField"] = "0";
return Options;
}
};
Modified: clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt?rev=361550&r1=361549&r2=361550&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt Thu May 23 13:29:04 2019
@@ -20,6 +20,7 @@ add_clang_library(clangTidyCERTModule
clangBasic
clangLex
clangTidy
+ clangTidyBugproneModule
clangTidyGoogleModule
clangTidyMiscModule
clangTidyPerformanceModule
Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=361550&r1=361549&r2=361550&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Thu May 23 13:29:04 2019
@@ -134,6 +134,11 @@ Improvements to clang-tidy
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
Modified: 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=361550&r1=361549&r2=361550&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst Thu May 23 13:29:04 2019
@@ -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 @@ temporary object into ``this`` (needs a
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.
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/cert-oop54-cpp.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cert-oop54-cpp.rst?rev=361550&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/cert-oop54-cpp.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/cert-oop54-cpp.rst Thu May 23 13:29:04 2019
@@ -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.
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=361550&r1=361549&r2=361550&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Thu May 23 13:29:04 2019
@@ -98,6 +98,7 @@ Clang-Tidy Checks
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>
Added: clang-tools-extra/trunk/test/clang-tidy/bugprone-unhandled-self-assignment-warn-only-if-this-has-suspicious-field.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-unhandled-self-assignment-warn-only-if-this-has-suspicious-field.cpp?rev=361550&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-unhandled-self-assignment-warn-only-if-this-has-suspicious-field.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-unhandled-self-assignment-warn-only-if-this-has-suspicious-field.cpp Thu May 23 13:29:04 2019
@@ -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;
+ }
+};
Added: clang-tools-extra/trunk/test/clang-tidy/cert-oop54-cpp.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cert-oop54-cpp.cpp?rev=361550&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/cert-oop54-cpp.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/cert-oop54-cpp.cpp Thu May 23 13:29:04 2019
@@ -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;
+};
More information about the cfe-commits
mailing list