https://github.com/vbvictor updated https://github.com/llvm/llvm-project/pull/121291
>From 37dce6a7ed0cca2e9819c24f4d176c43e3c9f2ac Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Sun, 29 Dec 2024 15:32:22 +0300 Subject: [PATCH 1/4] [clang-tidy] Add bugprone-reset-call check --- .../bugprone/BugproneTidyModule.cpp | 2 + .../clang-tidy/bugprone/CMakeLists.txt | 1 + .../clang-tidy/bugprone/ResetCallCheck.cpp | 133 +++++++++++ .../clang-tidy/bugprone/ResetCallCheck.h | 34 +++ clang-tools-extra/docs/ReleaseNotes.rst | 6 + .../clang-tidy/checks/bugprone/reset-call.rst | 33 +++ .../docs/clang-tidy/checks/list.rst | 1 + .../checkers/bugprone/reset-call.cpp | 215 ++++++++++++++++++ 8 files changed, 425 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/reset-call.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/reset-call.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 33ac65e715ce81..645958e47e22a5 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -57,6 +57,7 @@ #include "PosixReturnCheck.h" #include "RedundantBranchConditionCheck.h" #include "ReservedIdentifierCheck.h" +#include "ResetCallCheck.h" #include "ReturnConstRefFromParameterCheck.h" #include "SharedPtrArrayMismatchCheck.h" #include "SignalHandlerCheck.h" @@ -144,6 +145,7 @@ class BugproneModule : public ClangTidyModule { "bugprone-inaccurate-erase"); CheckFactories.registerCheck<IncorrectEnableIfCheck>( "bugprone-incorrect-enable-if"); + CheckFactories.registerCheck<ResetCallCheck>("bugprone-reset-call"); CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>( "bugprone-return-const-ref-from-parameter"); CheckFactories.registerCheck<SwitchMissingDefaultCaseCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index 13adad7c3dadbd..17ab5b27ec5550 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -52,6 +52,7 @@ add_clang_library(clangTidyBugproneModule STATIC PosixReturnCheck.cpp RedundantBranchConditionCheck.cpp ReservedIdentifierCheck.cpp + ResetCallCheck.cpp ReturnConstRefFromParameterCheck.cpp SharedPtrArrayMismatchCheck.cpp SignalHandlerCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp new file mode 100644 index 00000000000000..305ac8d51adf3e --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp @@ -0,0 +1,133 @@ +//===--- ResetCallCheck.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 "ResetCallCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +namespace { + +AST_MATCHER_P(CallExpr, everyArgumentMatches, + ast_matchers::internal::Matcher<Expr>, InnerMatcher) { + if (Node.getNumArgs() == 0) + return true; + + for (const auto *Arg : Node.arguments()) { + if (!InnerMatcher.matches(*Arg, Finder, Builder)) + return false; + } + return true; +} + +AST_MATCHER(CXXMethodDecl, hasOnlyDefaultArgs) { + if (Node.param_empty()) + return true; + + for (const auto *Param : Node.parameters()) { + if (!Param->hasDefaultArg()) + return false; + } + return true; +} + +} // namespace + +void ResetCallCheck::registerMatchers(MatchFinder *Finder) { + const auto IsSmartptr = hasAnyName("::std::unique_ptr", "::std::shared_ptr"); + + const auto ResetMethod = + cxxMethodDecl(hasName("reset"), hasOnlyDefaultArgs()); + + const auto TypeWithReset = + anyOf(cxxRecordDecl(hasMethod(ResetMethod)), + classTemplateSpecializationDecl( + hasSpecializedTemplate(classTemplateDecl(has(ResetMethod))))); + + const auto SmartptrWithBugproneReset = classTemplateSpecializationDecl( + IsSmartptr, + hasTemplateArgument( + 0, templateArgument(refersToType(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(TypeWithReset))))))); + + // Find a.reset() calls + Finder->addMatcher( + cxxMemberCallExpr(callee(memberExpr(member(hasName("reset")))), + everyArgumentMatches(cxxDefaultArgExpr()), + on(expr(hasType(SmartptrWithBugproneReset)))) + .bind("smartptrResetCall"), + this); + + // Find a->reset() calls + Finder->addMatcher( + cxxMemberCallExpr( + callee(memberExpr( + member(ResetMethod), + hasObjectExpression( + cxxOperatorCallExpr( + hasOverloadedOperatorName("->"), + hasArgument( + 0, expr(hasType( + classTemplateSpecializationDecl(IsSmartptr))))) + .bind("OpCall")))), + everyArgumentMatches(cxxDefaultArgExpr())) + .bind("objectResetCall"), + this); +} + +void ResetCallCheck::check(const MatchFinder::MatchResult &Result) { + const auto *SmartptrResetCall = + Result.Nodes.getNodeAs<CXXMemberCallExpr>("smartptrResetCall"); + const auto *ObjectResetCall = + Result.Nodes.getNodeAs<CXXMemberCallExpr>("objectResetCall"); + + if (SmartptrResetCall) { + const auto *Member = cast<MemberExpr>(SmartptrResetCall->getCallee()); + + auto DiagBuilder = diag(SmartptrResetCall->getBeginLoc(), + "bugprone 'reset()' call on smart pointer"); + + DiagBuilder << FixItHint::CreateReplacement( + SourceRange(Member->getOperatorLoc(), + Member->getOperatorLoc().getLocWithOffset(0)), + " ="); + + DiagBuilder << FixItHint::CreateReplacement( + SourceRange(Member->getMemberLoc(), SmartptrResetCall->getEndLoc()), + " nullptr"); + } + + if (ObjectResetCall) { + const auto *Arrow = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("OpCall"); + + const auto SmartptrSourceRange = + Lexer::getAsCharRange(Arrow->getArg(0)->getSourceRange(), + *Result.SourceManager, getLangOpts()); + + auto DiagBuilder = diag(ObjectResetCall->getBeginLoc(), + "bugprone 'reset()' call on smart pointer"); + + DiagBuilder << FixItHint::CreateInsertion(SmartptrSourceRange.getBegin(), + "(*"); + DiagBuilder << FixItHint::CreateInsertion(SmartptrSourceRange.getEnd(), + ")"); + + DiagBuilder << FixItHint::CreateReplacement( + CharSourceRange::getCharRange( + Arrow->getOperatorLoc(), + Arrow->getOperatorLoc().getLocWithOffset(2)), + "."); + } +} + +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.h b/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.h new file mode 100644 index 00000000000000..18f6ee4f6bed22 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.h @@ -0,0 +1,34 @@ +//===--- ResetCallCheck.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_RESETCALLCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RESETCALLCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::bugprone { + +/// Finds potentially erroneous calls to 'reset()' method on smart pointers when +/// the pointee type also has a 'reset()' method +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/reset-call.html +class ResetCallCheck : public ClangTidyCheck { +public: + ResetCallCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RESETCALLCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index fa3a8e577a33ad..d125afef0bb039 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -148,6 +148,12 @@ New checks Finds nondeterministic usages of pointers in unordered containers. +- New :doc:`bugprone-reset-call + <clang-tidy/checks/bugprone/reset-call>` check. + + Finds potentially erroneous calls to ``reset()`` method on smart pointers when + the pointee type also has a ``reset()`` method. + - New :doc:`bugprone-tagged-union-member-count <clang-tidy/checks/bugprone/tagged-union-member-count>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/reset-call.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/reset-call.rst new file mode 100644 index 00000000000000..87cf955b26b648 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/reset-call.rst @@ -0,0 +1,33 @@ +.. title:: clang-tidy - bugprone-reset-call + +bugprone-reset-call +=================== + +Finds calls to ``reset()`` method on smart pointers where the pointee type +also has a ``reset()`` method, which makes it easy to accidentally +make the pointer null when intending to reset the underlying object. + +.. code-block:: c++ + + struct Resettable { + void reset() { /* Own reset logic */ } + }; + + auto ptr = std::make_unique<Resettable>(); + + ptr->reset(); // Calls underlying reset method + ptr.reset(); // Makes the pointer null + +Both calls are valid C++ code, but the second one might not be +what the developer intended, as it destroys the pointed-to object +rather than resetting its state. +It's easy to make such typo because the difference between ``.`` and ``->`` is really small. + +The recommended approach is to make the intent explicit by using either member access or direct assignment: + +.. code-block:: c++ + + std::unique_ptr<Resettable> ptr = std::make_unique<Resettable>(); + + (*ptr).reset(); // Clearly calls underlying reset method + ptr = nullptr; // Clearly makes the pointer null \ No newline at end of file diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 4d8853a0f6d86c..9ca866da74666d 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -124,6 +124,7 @@ Clang-Tidy Checks :doc:`bugprone-posix-return <bugprone/posix-return>`, "Yes" :doc:`bugprone-redundant-branch-condition <bugprone/redundant-branch-condition>`, "Yes" :doc:`bugprone-reserved-identifier <bugprone/reserved-identifier>`, "Yes" + :doc:`bugprone-reset-call <bugprone/reset-call>`, "Yes" :doc:`bugprone-return-const-ref-from-parameter <bugprone/return-const-ref-from-parameter>`, :doc:`bugprone-shared-ptr-array-mismatch <bugprone/shared-ptr-array-mismatch>`, "Yes" :doc:`bugprone-signal-handler <bugprone/signal-handler>`, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/reset-call.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/reset-call.cpp new file mode 100644 index 00000000000000..d98f4b463840c7 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/reset-call.cpp @@ -0,0 +1,215 @@ +// RUN: %check_clang_tidy %s bugprone-reset-call %t + +namespace std { + +template <typename T> +struct unique_ptr { + T& operator*() const; + T* operator->() const; + void reset(T* p = nullptr); +}; + +template <typename T> +struct shared_ptr { + T& operator*() const; + T* operator->() const; + void reset(); + void reset(T*); +}; + +} // namespace std + +struct Resettable { + void reset(); + void doSomething(); +}; + +struct ResettableWithParam { + void reset(int a); + void doSomething(); +}; + +struct ResettableWithDefaultParams { + void reset(int a = 0, double b = 0.0); + void doSomething(); +}; + +struct NonResettable { + void doSomething(); +}; + +void Positive() { + std::unique_ptr<Resettable> u; + u.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: u = nullptr; + u->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: (*u).reset(); + + std::shared_ptr<Resettable> s; + s.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: s = nullptr; + s->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: (*s).reset(); + + std::unique_ptr<std::unique_ptr<int>> uu_ptr; + uu_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: uu_ptr = nullptr; + uu_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: (*uu_ptr).reset(); + + std::unique_ptr<std::shared_ptr<int>> su_ptr; + su_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: su_ptr = nullptr; + su_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: (*su_ptr).reset(); + + std::unique_ptr<ResettableWithDefaultParams> rd; + rd.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: rd = nullptr; + rd->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: (*rd).reset(); + + std::unique_ptr<std::shared_ptr<std::unique_ptr<Resettable>>> nested_ptr; + nested_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: nested_ptr = nullptr; + nested_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: (*nested_ptr).reset(); + (*nested_ptr).reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: (*nested_ptr) = nullptr; + (*nested_ptr)->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: (*(*nested_ptr)).reset(); + (**nested_ptr).reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: (**nested_ptr) = nullptr; + (**nested_ptr)->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: (*(**nested_ptr)).reset(); +} + +void Negative() { + std::unique_ptr<Resettable> u_ptr; + u_ptr.reset(nullptr); + u_ptr->doSomething(); + + std::shared_ptr<Resettable> s_ptr; + s_ptr.reset(nullptr); + s_ptr->doSomething(); + + Resettable* raw_ptr; + raw_ptr->reset(); + raw_ptr->doSomething(); + + Resettable resettable; + resettable.reset(); + resettable.doSomething(); + + std::unique_ptr<ResettableWithParam> u_ptr_param; + u_ptr_param.reset(); + u_ptr_param.reset(nullptr); + u_ptr_param->reset(0); + + std::unique_ptr<NonResettable> u_ptr_no_reset; + u_ptr_no_reset.reset(); + + std::shared_ptr<ResettableWithParam> s_ptr_param; + s_ptr_param.reset(); + s_ptr_param->reset(0); + s_ptr_param->doSomething(); + + std::shared_ptr<NonResettable> s_ptr_no_reset; + s_ptr_no_reset.reset(); + + std::unique_ptr<ResettableWithDefaultParams> u_ptr_default_params; + u_ptr_default_params.reset(nullptr); + u_ptr_default_params->reset(1); + u_ptr_default_params->reset(1, 2.0); +} + +template <typename T> +void TemplatePositiveTest() { + std::unique_ptr<T> u_ptr; + + u_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: u_ptr = nullptr; + u_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: (*u_ptr).reset(); + + std::shared_ptr<T> s_ptr; + s_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: s_ptr = nullptr; + s_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: (*s_ptr).reset(); +} + +template <typename T> +void TemplatNegativeTestTypeWithReset() { + std::unique_ptr<T> u_ptr; + u_ptr.reset(); + u_ptr->reset(0); + + std::shared_ptr<T> s_ptr; + s_ptr.reset(); + s_ptr->reset(0); +} + +template <typename T> +void TemplatNegativeTestTypeWithoutReset() { + std::unique_ptr<T> u_ptr; + u_ptr.reset(); + + std::unique_ptr<T> s_ptr; + s_ptr.reset(); +} + +void instantiate() { + TemplatePositiveTest<Resettable>(); + TemplatePositiveTest<std::unique_ptr<int>>(); + TemplatePositiveTest<std::shared_ptr<int>>(); + TemplatNegativeTestTypeWithReset<ResettableWithParam>(); + TemplatNegativeTestTypeWithoutReset<NonResettable>(); +} + +struct S { + void foo() { + u_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: u_ptr = nullptr; + u_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: (*u_ptr).reset(); + u_ptr.reset(nullptr); + u_ptr->doSomething(); + + s_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: s_ptr = nullptr; + s_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: (*s_ptr).reset(); + s_ptr.reset(nullptr); + + ptr.reset(); + } + + std::unique_ptr<Resettable> u_ptr; + std::unique_ptr<std::shared_ptr<int>> s_ptr; + std::unique_ptr<NonResettable> ptr; +}; >From b42070a157050408488e9d3d5b98c610ea4abc95 Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Mon, 30 Dec 2024 00:54:30 +0300 Subject: [PATCH 2/4] [clang-tidy] bugprone-smartptr-reset-ambiguous-call fix: rename check to bugprone-smartptr-reset-ambiguous-call, make first statement in docs same as in Release Notes --- .../clang-tidy/bugprone/BugproneTidyModule.cpp | 5 +++-- .../clang-tidy/bugprone/CMakeLists.txt | 2 +- ...eck.cpp => SmartptrResetAmbiguousCallCheck.cpp} | 9 +++++---- ...llCheck.h => SmartptrResetAmbiguousCallCheck.h} | 14 +++++++------- clang-tools-extra/docs/ReleaseNotes.rst | 4 ++-- ...-call.rst => smartptr-reset-ambiguous-call.rst} | 11 ++++++----- clang-tools-extra/docs/clang-tidy/checks/list.rst | 2 +- ...-call.cpp => smartptr-reset-ambiguous-call.cpp} | 2 +- 8 files changed, 26 insertions(+), 23 deletions(-) rename clang-tools-extra/clang-tidy/bugprone/{ResetCallCheck.cpp => SmartptrResetAmbiguousCallCheck.cpp} (93%) rename clang-tools-extra/clang-tidy/bugprone/{ResetCallCheck.h => SmartptrResetAmbiguousCallCheck.h} (61%) rename clang-tools-extra/docs/clang-tidy/checks/bugprone/{reset-call.rst => smartptr-reset-ambiguous-call.rst} (71%) rename clang-tools-extra/test/clang-tidy/checkers/bugprone/{reset-call.cpp => smartptr-reset-ambiguous-call.cpp} (98%) diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 645958e47e22a5..0870d86420068f 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -57,13 +57,13 @@ #include "PosixReturnCheck.h" #include "RedundantBranchConditionCheck.h" #include "ReservedIdentifierCheck.h" -#include "ResetCallCheck.h" #include "ReturnConstRefFromParameterCheck.h" #include "SharedPtrArrayMismatchCheck.h" #include "SignalHandlerCheck.h" #include "SignedCharMisuseCheck.h" #include "SizeofContainerCheck.h" #include "SizeofExpressionCheck.h" +#include "SmartptrResetAmbiguousCallCheck.h" #include "SpuriouslyWakeUpFunctionsCheck.h" #include "StandaloneEmptyCheck.h" #include "StringConstructorCheck.h" @@ -145,7 +145,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-inaccurate-erase"); CheckFactories.registerCheck<IncorrectEnableIfCheck>( "bugprone-incorrect-enable-if"); - CheckFactories.registerCheck<ResetCallCheck>("bugprone-reset-call"); + CheckFactories.registerCheck<SmartptrResetAmbiguousCallCheck>( + "bugprone-smartptr-reset-ambiguous-call"); CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>( "bugprone-return-const-ref-from-parameter"); CheckFactories.registerCheck<SwitchMissingDefaultCaseCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index 17ab5b27ec5550..feaa5d2553aef2 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -52,7 +52,6 @@ add_clang_library(clangTidyBugproneModule STATIC PosixReturnCheck.cpp RedundantBranchConditionCheck.cpp ReservedIdentifierCheck.cpp - ResetCallCheck.cpp ReturnConstRefFromParameterCheck.cpp SharedPtrArrayMismatchCheck.cpp SignalHandlerCheck.cpp @@ -60,6 +59,7 @@ add_clang_library(clangTidyBugproneModule STATIC SizeofContainerCheck.cpp SizeofExpressionCheck.cpp SmartPtrArrayMismatchCheck.cpp + SmartptrResetAmbiguousCallCheck.cpp SpuriouslyWakeUpFunctionsCheck.cpp StandaloneEmptyCheck.cpp StringConstructorCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp similarity index 93% rename from clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp rename to clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp index 305ac8d51adf3e..03a0eab89cb8a7 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp @@ -1,4 +1,4 @@ -//===--- ResetCallCheck.cpp - clang-tidy ----------------------------------===// +//===--- SmartptrResetAmbiguousCallCheck.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. @@ -6,7 +6,7 @@ // //===----------------------------------------------------------------------===// -#include "ResetCallCheck.h" +#include "SmartptrResetAmbiguousCallCheck.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" @@ -43,7 +43,7 @@ AST_MATCHER(CXXMethodDecl, hasOnlyDefaultArgs) { } // namespace -void ResetCallCheck::registerMatchers(MatchFinder *Finder) { +void SmartptrResetAmbiguousCallCheck::registerMatchers(MatchFinder *Finder) { const auto IsSmartptr = hasAnyName("::std::unique_ptr", "::std::shared_ptr"); const auto ResetMethod = @@ -85,7 +85,8 @@ void ResetCallCheck::registerMatchers(MatchFinder *Finder) { this); } -void ResetCallCheck::check(const MatchFinder::MatchResult &Result) { +void SmartptrResetAmbiguousCallCheck::check( + const MatchFinder::MatchResult &Result) { const auto *SmartptrResetCall = Result.Nodes.getNodeAs<CXXMemberCallExpr>("smartptrResetCall"); const auto *ObjectResetCall = diff --git a/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.h b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h similarity index 61% rename from clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.h rename to clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h index 18f6ee4f6bed22..bac8df9469bbf3 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h @@ -1,4 +1,4 @@ -//===--- ResetCallCheck.h - clang-tidy --------------------------*- C++ -*-===// +//===--- SmartptrResetAmbiguousCallCheck.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. @@ -6,8 +6,8 @@ // //===----------------------------------------------------------------------===// -#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RESETCALLCHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RESETCALLCHECK_H +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SMARTPTRRESETAMBIGUOUSCALLCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SMARTPTRRESETAMBIGUOUSCALLCHECK_H #include "../ClangTidyCheck.h" @@ -17,10 +17,10 @@ namespace clang::tidy::bugprone { /// the pointee type also has a 'reset()' method /// /// For the user-facing documentation see: -/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/reset-call.html -class ResetCallCheck : public ClangTidyCheck { +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.html +class SmartptrResetAmbiguousCallCheck : public ClangTidyCheck { public: - ResetCallCheck(StringRef Name, ClangTidyContext *Context) + SmartptrResetAmbiguousCallCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context) {} void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; @@ -31,4 +31,4 @@ class ResetCallCheck : public ClangTidyCheck { } // namespace clang::tidy::bugprone -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RESETCALLCHECK_H +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SMARTPTRRESETAMBIGUOUSCALLCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index d125afef0bb039..f25bd338c61c9c 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -148,8 +148,8 @@ New checks Finds nondeterministic usages of pointers in unordered containers. -- New :doc:`bugprone-reset-call - <clang-tidy/checks/bugprone/reset-call>` check. +- New :doc:`bugprone-smartptr-reset-ambiguous-call + <clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call>` check. Finds potentially erroneous calls to ``reset()`` method on smart pointers when the pointee type also has a ``reset()`` method. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/reset-call.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.rst similarity index 71% rename from clang-tools-extra/docs/clang-tidy/checks/bugprone/reset-call.rst rename to clang-tools-extra/docs/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.rst index 87cf955b26b648..8508eb6b592f6a 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/reset-call.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.rst @@ -1,10 +1,11 @@ -.. title:: clang-tidy - bugprone-reset-call +.. title:: clang-tidy - bugprone-smartptr-reset-ambiguous-call -bugprone-reset-call -=================== +bugprone-smartptr-reset-ambiguous-call +====================================== -Finds calls to ``reset()`` method on smart pointers where the pointee type -also has a ``reset()`` method, which makes it easy to accidentally +Finds potentially erroneous calls to ``reset()`` method on +smart pointers when the pointee type also has a ``reset()`` method. +Having ``reset()`` method in both classes makes it easy to accidentally make the pointer null when intending to reset the underlying object. .. code-block:: c++ diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 9ca866da74666d..fdf7663c0fd915 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -124,13 +124,13 @@ Clang-Tidy Checks :doc:`bugprone-posix-return <bugprone/posix-return>`, "Yes" :doc:`bugprone-redundant-branch-condition <bugprone/redundant-branch-condition>`, "Yes" :doc:`bugprone-reserved-identifier <bugprone/reserved-identifier>`, "Yes" - :doc:`bugprone-reset-call <bugprone/reset-call>`, "Yes" :doc:`bugprone-return-const-ref-from-parameter <bugprone/return-const-ref-from-parameter>`, :doc:`bugprone-shared-ptr-array-mismatch <bugprone/shared-ptr-array-mismatch>`, "Yes" :doc:`bugprone-signal-handler <bugprone/signal-handler>`, :doc:`bugprone-signed-char-misuse <bugprone/signed-char-misuse>`, :doc:`bugprone-sizeof-container <bugprone/sizeof-container>`, :doc:`bugprone-sizeof-expression <bugprone/sizeof-expression>`, + :doc:`bugprone-smartptr-reset-ambiguous-call <bugprone/smartptr-reset-ambiguous-call>`, "Yes" :doc:`bugprone-spuriously-wake-up-functions <bugprone/spuriously-wake-up-functions>`, :doc:`bugprone-standalone-empty <bugprone/standalone-empty>`, "Yes" :doc:`bugprone-string-constructor <bugprone/string-constructor>`, "Yes" diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/reset-call.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call.cpp similarity index 98% rename from clang-tools-extra/test/clang-tidy/checkers/bugprone/reset-call.cpp rename to clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call.cpp index d98f4b463840c7..5d1585049cd762 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/reset-call.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call.cpp @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy %s bugprone-reset-call %t +// RUN: %check_clang_tidy %s bugprone-smartptr-reset-ambiguous-call %t namespace std { >From 5bcef62a02d4c7fafaf834e1c57f269e16b9302e Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Mon, 30 Dec 2024 02:04:02 +0300 Subject: [PATCH 3/4] [clang-tidy] bugprone-smartptr-reset-ambiguous-call refactor: removed parentheses in method name from docs --- .../clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h | 4 ++-- clang-tools-extra/docs/ReleaseNotes.rst | 4 ++-- .../checks/bugprone/smartptr-reset-ambiguous-call.rst | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h index bac8df9469bbf3..7867debbdbf1a5 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h @@ -13,8 +13,8 @@ namespace clang::tidy::bugprone { -/// Finds potentially erroneous calls to 'reset()' method on smart pointers when -/// the pointee type also has a 'reset()' method +/// Finds potentially erroneous calls to 'reset' method on smart pointers when +/// the pointee type also has a 'reset' method /// /// For the user-facing documentation see: /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.html diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index f25bd338c61c9c..6c31163c24b90e 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -151,8 +151,8 @@ New checks - New :doc:`bugprone-smartptr-reset-ambiguous-call <clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call>` check. - Finds potentially erroneous calls to ``reset()`` method on smart pointers when - the pointee type also has a ``reset()`` method. + Finds potentially erroneous calls to ``reset`` method on smart pointers when + the pointee type also has a ``reset`` method. - New :doc:`bugprone-tagged-union-member-count <clang-tidy/checks/bugprone/tagged-union-member-count>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.rst index 8508eb6b592f6a..ff306bd02e8ac3 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.rst @@ -3,9 +3,9 @@ bugprone-smartptr-reset-ambiguous-call ====================================== -Finds potentially erroneous calls to ``reset()`` method on -smart pointers when the pointee type also has a ``reset()`` method. -Having ``reset()`` method in both classes makes it easy to accidentally +Finds potentially erroneous calls to ``reset`` method on +smart pointers when the pointee type also has a ``reset`` method. +Having ``reset`` method in both classes makes it easy to accidentally make the pointer null when intending to reset the underlying object. .. code-block:: c++ >From 02ba990bcdca6e971a70dcb24e9d78e426350f06 Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Thu, 9 Jan 2025 19:55:11 +0300 Subject: [PATCH 4/4] [clang-tidy] bugprone-smartptr-reset-ambiguous-call. feat: added option for custom smart pointers. fix: pr comments --- .../bugprone/BugproneTidyModule.cpp | 4 +- .../SmartptrResetAmbiguousCallCheck.cpp | 34 +++++++--- .../SmartptrResetAmbiguousCallCheck.h | 7 +- .../smartptr-reset-ambiguous-call.rst | 21 ++++-- ...r-reset-ambiguous-call-custom-pointers.cpp | 68 +++++++++++++++++++ 5 files changed, 116 insertions(+), 18 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call-custom-pointers.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 0870d86420068f..1d317e1daf0cd3 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -145,8 +145,6 @@ class BugproneModule : public ClangTidyModule { "bugprone-inaccurate-erase"); CheckFactories.registerCheck<IncorrectEnableIfCheck>( "bugprone-incorrect-enable-if"); - CheckFactories.registerCheck<SmartptrResetAmbiguousCallCheck>( - "bugprone-smartptr-reset-ambiguous-call"); CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>( "bugprone-return-const-ref-from-parameter"); CheckFactories.registerCheck<SwitchMissingDefaultCaseCheck>( @@ -207,6 +205,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-sizeof-container"); CheckFactories.registerCheck<SizeofExpressionCheck>( "bugprone-sizeof-expression"); + CheckFactories.registerCheck<SmartptrResetAmbiguousCallCheck>( + "bugprone-smartptr-reset-ambiguous-call"); CheckFactories.registerCheck<SpuriouslyWakeUpFunctionsCheck>( "bugprone-spuriously-wake-up-functions"); CheckFactories.registerCheck<StandaloneEmptyCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp index 03a0eab89cb8a7..44c7d114c3ddb8 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "SmartptrResetAmbiguousCallCheck.h" +#include "../utils/OptionsUtils.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" @@ -30,7 +31,7 @@ AST_MATCHER_P(CallExpr, everyArgumentMatches, return true; } -AST_MATCHER(CXXMethodDecl, hasOnlyDefaultArgs) { +AST_MATCHER(CXXMethodDecl, hasOnlyDefaultParameters) { if (Node.param_empty()) return true; @@ -41,13 +42,26 @@ AST_MATCHER(CXXMethodDecl, hasOnlyDefaultArgs) { return true; } +const auto DefaultSmartPointers = "::std::shared_ptr; ::std::unique_ptr"; } // namespace +SmartptrResetAmbiguousCallCheck::SmartptrResetAmbiguousCallCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + SmartPointers(utils::options::parseStringList( + Options.get("SmartPointers", DefaultSmartPointers))) {} + +void SmartptrResetAmbiguousCallCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "SmartPointers", + utils::options::serializeStringList(SmartPointers)); +} + void SmartptrResetAmbiguousCallCheck::registerMatchers(MatchFinder *Finder) { - const auto IsSmartptr = hasAnyName("::std::unique_ptr", "::std::shared_ptr"); + const auto IsSmartptr = hasAnyName(SmartPointers); const auto ResetMethod = - cxxMethodDecl(hasName("reset"), hasOnlyDefaultArgs()); + cxxMethodDecl(hasName("reset"), hasOnlyDefaultParameters()); const auto TypeWithReset = anyOf(cxxRecordDecl(hasMethod(ResetMethod)), @@ -87,12 +101,9 @@ void SmartptrResetAmbiguousCallCheck::registerMatchers(MatchFinder *Finder) { void SmartptrResetAmbiguousCallCheck::check( const MatchFinder::MatchResult &Result) { - const auto *SmartptrResetCall = - Result.Nodes.getNodeAs<CXXMemberCallExpr>("smartptrResetCall"); - const auto *ObjectResetCall = - Result.Nodes.getNodeAs<CXXMemberCallExpr>("objectResetCall"); - if (SmartptrResetCall) { + if (const auto *SmartptrResetCall = + Result.Nodes.getNodeAs<CXXMemberCallExpr>("smartptrResetCall")) { const auto *Member = cast<MemberExpr>(SmartptrResetCall->getCallee()); auto DiagBuilder = diag(SmartptrResetCall->getBeginLoc(), @@ -106,12 +117,15 @@ void SmartptrResetAmbiguousCallCheck::check( DiagBuilder << FixItHint::CreateReplacement( SourceRange(Member->getMemberLoc(), SmartptrResetCall->getEndLoc()), " nullptr"); + + return; } - if (ObjectResetCall) { + if (const auto *ObjectResetCall = + Result.Nodes.getNodeAs<CXXMemberCallExpr>("objectResetCall")) { const auto *Arrow = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("OpCall"); - const auto SmartptrSourceRange = + const CharSourceRange SmartptrSourceRange = Lexer::getAsCharRange(Arrow->getArg(0)->getSourceRange(), *Result.SourceManager, getLangOpts()); diff --git a/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h index 7867debbdbf1a5..474bcf18045817 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h @@ -20,13 +20,16 @@ namespace clang::tidy::bugprone { /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.html class SmartptrResetAmbiguousCallCheck : public ClangTidyCheck { public: - SmartptrResetAmbiguousCallCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + SmartptrResetAmbiguousCallCheck(StringRef Name, ClangTidyContext *Context); void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; } + +private: + const std::vector<StringRef> SmartPointers; }; } // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.rst index ff306bd02e8ac3..2fa7b511ac9f29 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.rst @@ -5,7 +5,7 @@ bugprone-smartptr-reset-ambiguous-call Finds potentially erroneous calls to ``reset`` method on smart pointers when the pointee type also has a ``reset`` method. -Having ``reset`` method in both classes makes it easy to accidentally +Having a ``reset`` method in both classes makes it easy to accidentally make the pointer null when intending to reset the underlying object. .. code-block:: c++ @@ -22,13 +22,26 @@ make the pointer null when intending to reset the underlying object. Both calls are valid C++ code, but the second one might not be what the developer intended, as it destroys the pointed-to object rather than resetting its state. -It's easy to make such typo because the difference between ``.`` and ``->`` is really small. +It's easy to make such a typo because the difference +between ``.`` and ``->`` is really small. -The recommended approach is to make the intent explicit by using either member access or direct assignment: +The recommended approach is to make the intent explicit by +using either member access or direct assignment: .. code-block:: c++ std::unique_ptr<Resettable> ptr = std::make_unique<Resettable>(); (*ptr).reset(); // Clearly calls underlying reset method - ptr = nullptr; // Clearly makes the pointer null \ No newline at end of file + ptr = nullptr; // Clearly makes the pointer null + +The default smart pointers that are considered are ``std::unique_ptr``, +``std::shared_ptr``. To specify other smart pointers or +other classes use the :option:`SmartPointers` option. + +Options +------- + +.. option:: SmartPointers + + Semicolon-separated list of class names of custom smart pointers. diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call-custom-pointers.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call-custom-pointers.cpp new file mode 100644 index 00000000000000..b4ed0b922282e5 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call-custom-pointers.cpp @@ -0,0 +1,68 @@ +// RUN: %check_clang_tidy %s bugprone-smartptr-reset-ambiguous-call %t \ +// RUN: -config='{CheckOptions: \ +// RUN: {bugprone-smartptr-reset-ambiguous-call.SmartPointers: "::std::unique_ptr;boost::shared_ptr"}}' \ +// RUN: -- + +namespace std { + +template <typename T> +struct unique_ptr { + T& operator*() const; + T* operator->() const; + void reset(T* p = nullptr); +}; + +template <typename T> +struct shared_ptr { + T& operator*() const; + T* operator->() const; + void reset(); + void reset(T*); +}; + +} // namespace std + +namespace boost { + +template <typename T> +struct shared_ptr { + T& operator*() const; + T* operator->() const; + void reset(); +}; + +} // namespace boost + +struct Resettable { + void reset(); + void doSomething(); +}; + +void Positive() { + std::unique_ptr<Resettable> u; + u.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: u = nullptr; + u->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: (*u).reset(); + + boost::shared_ptr<Resettable> s; + s.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: s = nullptr; + s->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: bugprone 'reset()' call on smart pointer + // CHECK-FIXES: (*s).reset(); +} + +void Negative() { + std::shared_ptr<Resettable> s_ptr; + s_ptr.reset(); + s_ptr->reset(); + s_ptr->doSomething(); + + std::unique_ptr<Resettable> u_ptr; + u_ptr.reset(nullptr); + u_ptr->doSomething(); +} \ No newline at end of file _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits