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; +}; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
