https://github.com/vbvictor updated https://github.com/llvm/llvm-project/pull/121291
>From 42e03bb9cc9bd815476b0a3f06ac5f58826e3708 Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Fri, 31 Jan 2025 19:29:05 +0300 Subject: [PATCH 1/2] [clang-tidy] add new check bugprone-reset-ambiguous-call --- .../bugprone/BugproneTidyModule.cpp | 3 + .../clang-tidy/bugprone/CMakeLists.txt | 1 + .../SmartptrResetAmbiguousCallCheck.cpp | 146 +++++++++++ .../SmartptrResetAmbiguousCallCheck.h | 37 +++ clang-tools-extra/docs/ReleaseNotes.rst | 6 + .../smartptr-reset-ambiguous-call.rst | 47 ++++ .../docs/clang-tidy/checks/list.rst | 1 + ...r-reset-ambiguous-call-custom-pointers.cpp | 72 ++++++ .../smartptr-reset-ambiguous-call.cpp | 239 ++++++++++++++++++ 9 files changed, 552 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call-custom-pointers.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index c5f0b5b28418f8..a01d0e384ab737 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -64,6 +64,7 @@ #include "SignedCharMisuseCheck.h" #include "SizeofContainerCheck.h" #include "SizeofExpressionCheck.h" +#include "SmartptrResetAmbiguousCallCheck.h" #include "SpuriouslyWakeUpFunctionsCheck.h" #include "StandaloneEmptyCheck.h" #include "StringConstructorCheck.h" @@ -207,6 +208,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/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index e8309c68b7fcaa..7fd6f4994f5375 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -65,6 +65,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/SmartptrResetAmbiguousCallCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp new file mode 100644 index 00000000000000..326a5665179d79 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp @@ -0,0 +1,146 @@ +//===--- 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. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "SmartptrResetAmbiguousCallCheck.h" +#include "../utils/OptionsUtils.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) { + for (const auto *Arg : Node.arguments()) { + if (!InnerMatcher.matches(*Arg, Finder, Builder)) + return false; + } + + return true; +} + +AST_MATCHER(CXXMethodDecl, hasOnlyDefaultParameters) { + for (const auto *Param : Node.parameters()) { + if (!Param->hasDefaultArg()) + return false; + } + + 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(SmartPointers); + + const auto ResetMethod = + cxxMethodDecl(hasName("reset"), hasOnlyDefaultParameters()); + + 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 SmartptrResetAmbiguousCallCheck::check( + const MatchFinder::MatchResult &Result) { + + if (const auto *SmartptrResetCall = + Result.Nodes.getNodeAs<CXXMemberCallExpr>("smartptrResetCall")) { + const auto *Member = cast<MemberExpr>(SmartptrResetCall->getCallee()); + + diag(SmartptrResetCall->getBeginLoc(), + "be explicit when calling 'reset()' on a smart pointer with a " + "pointee that has a 'reset()' method"); + + diag(SmartptrResetCall->getBeginLoc(), "assign the pointer to 'nullptr'", + DiagnosticIDs::Note) + << FixItHint::CreateReplacement( + SourceRange(Member->getOperatorLoc(), + Member->getOperatorLoc().getLocWithOffset(0)), + " =") + << FixItHint::CreateReplacement( + SourceRange(Member->getMemberLoc(), + SmartptrResetCall->getEndLoc()), + " nullptr"); + return; + } + + if (const auto *ObjectResetCall = + Result.Nodes.getNodeAs<CXXMemberCallExpr>("objectResetCall")) { + const auto *Arrow = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("OpCall"); + + const CharSourceRange SmartptrSourceRange = + Lexer::getAsCharRange(Arrow->getArg(0)->getSourceRange(), + *Result.SourceManager, getLangOpts()); + + diag(ObjectResetCall->getBeginLoc(), + "be explicit when calling 'reset()' on a pointee of a smart pointer"); + + diag(ObjectResetCall->getBeginLoc(), + "use dereference to call 'reset' method of the pointee", + DiagnosticIDs::Note) + << FixItHint::CreateInsertion(SmartptrSourceRange.getBegin(), "(*") + << FixItHint::CreateInsertion(SmartptrSourceRange.getEnd(), ")") + << FixItHint::CreateReplacement( + CharSourceRange::getCharRange( + Arrow->getOperatorLoc(), + Arrow->getOperatorLoc().getLocWithOffset(2)), + "."); + } +} + +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h new file mode 100644 index 00000000000000..474bcf18045817 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h @@ -0,0 +1,37 @@ +//===--- 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. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SMARTPTRRESETAMBIGUOUSCALLCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SMARTPTRRESETAMBIGUOUSCALLCHECK_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/smartptr-reset-ambiguous-call.html +class SmartptrResetAmbiguousCallCheck : public ClangTidyCheck { +public: + 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 + +#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 3bddeeda06e06b..ef48acf958c9e7 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -91,6 +91,12 @@ Improvements to clang-tidy 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. + New check aliases ^^^^^^^^^^^^^^^^^ 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 new file mode 100644 index 00000000000000..263105e4208132 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.rst @@ -0,0 +1,47 @@ +.. title:: clang-tidy - bugprone-smartptr-reset-ambiguous-call + +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 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++ + + 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 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: + +.. 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 + +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. + Default value is `::std::unique_ptr;::std::shared_ptr`. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 7b9b905ef76715..ec6dd26b789772 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -132,6 +132,7 @@ Clang-Tidy Checks :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/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..bcb2498bd2043e --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call-custom-pointers.cpp @@ -0,0 +1,72 @@ +// 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: --fix-notes -- + +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: be explicit when calling 'reset()' on a smart pointer with a pointee that has a 'reset()' method + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: assign the pointer to 'nullptr' + // CHECK-FIXES: u = nullptr; + u->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: be explicit when calling 'reset()' on a pointee of a smart pointer + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: use dereference to call 'reset' method of the pointee + // CHECK-FIXES: (*u).reset(); + + boost::shared_ptr<Resettable> s; + s.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: be explicit when calling 'reset()' on a smart pointer with a pointee that has a 'reset()' method + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: assign the pointer to 'nullptr' + // CHECK-FIXES: s = nullptr; + s->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: be explicit when calling 'reset()' on a pointee of a smart pointer + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: use dereference to call 'reset' method of the pointee + // 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 diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call.cpp new file mode 100644 index 00000000000000..4227733404f6bf --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call.cpp @@ -0,0 +1,239 @@ +// RUN: %check_clang_tidy %s bugprone-smartptr-reset-ambiguous-call %t --fix-notes -- + +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: be explicit when calling 'reset()' on a smart pointer with a pointee that has a 'reset()' method + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: assign the pointer to 'nullptr' + // CHECK-FIXES: u = nullptr; + u->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: be explicit when calling 'reset()' on a pointee of a smart pointer + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: use dereference to call 'reset' method of the pointee + // CHECK-FIXES: (*u).reset(); + + std::shared_ptr<Resettable> s; + s.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: be explicit when calling 'reset()' on a smart pointer with a pointee that has a 'reset()' method + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: assign the pointer to 'nullptr' + // CHECK-FIXES: s = nullptr; + s->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: be explicit when calling 'reset()' on a pointee of a smart pointer + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: use dereference to call 'reset' method of the pointee + // CHECK-FIXES: (*s).reset(); + + std::unique_ptr<std::unique_ptr<int>> uu_ptr; + uu_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: be explicit when calling 'reset()' on a smart pointer with a pointee that has a 'reset()' method + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: assign the pointer to 'nullptr' + // CHECK-FIXES: uu_ptr = nullptr; + uu_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: be explicit when calling 'reset()' on a pointee of a smart pointer + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: use dereference to call 'reset' method of the pointee + // CHECK-FIXES: (*uu_ptr).reset(); + + std::unique_ptr<std::shared_ptr<int>> su_ptr; + su_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: be explicit when calling 'reset()' on a smart pointer with a pointee that has a 'reset()' method + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: assign the pointer to 'nullptr' + // CHECK-FIXES: su_ptr = nullptr; + su_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: be explicit when calling 'reset()' on a pointee of a smart pointer + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: use dereference to call 'reset' method of the pointee + // CHECK-FIXES: (*su_ptr).reset(); + + std::unique_ptr<ResettableWithDefaultParams> rd; + rd.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: be explicit when calling 'reset()' on a smart pointer with a pointee that has a 'reset()' method + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: assign the pointer to 'nullptr' + // CHECK-FIXES: rd = nullptr; + rd->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: be explicit when calling 'reset()' on a pointee of a smart pointer + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: use dereference to call 'reset' method of the pointee + // 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: be explicit when calling 'reset()' on a smart pointer with a pointee that has a 'reset()' method + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: assign the pointer to 'nullptr' + // CHECK-FIXES: nested_ptr = nullptr; + nested_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: be explicit when calling 'reset()' on a pointee of a smart pointer + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: use dereference to call 'reset' method of the pointee + // CHECK-FIXES: (*nested_ptr).reset(); + (*nested_ptr).reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: be explicit when calling 'reset()' on a smart pointer with a pointee that has a 'reset()' method + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: assign the pointer to 'nullptr' + // CHECK-FIXES: (*nested_ptr) = nullptr; + (*nested_ptr)->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: be explicit when calling 'reset()' on a pointee of a smart pointer + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: use dereference to call 'reset' method of the pointee + // CHECK-FIXES: (*(*nested_ptr)).reset(); + (**nested_ptr).reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: be explicit when calling 'reset()' on a smart pointer with a pointee that has a 'reset()' method + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: assign the pointer to 'nullptr' + // CHECK-FIXES: (**nested_ptr) = nullptr; + (**nested_ptr)->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: be explicit when calling 'reset()' on a pointee of a smart pointer + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: use dereference to call 'reset' method of the pointee + // 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: be explicit when calling 'reset()' on a smart pointer with a pointee that has a 'reset()' method + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: assign the pointer to 'nullptr' + // CHECK-FIXES: u_ptr = nullptr; + u_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: be explicit when calling 'reset()' on a pointee of a smart pointer + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: use dereference to call 'reset' method of the pointee + // CHECK-FIXES: (*u_ptr).reset(); + + std::shared_ptr<T> s_ptr; + s_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: be explicit when calling 'reset()' on a smart pointer with a pointee that has a 'reset()' method + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: assign the pointer to 'nullptr' + // CHECK-FIXES: s_ptr = nullptr; + s_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: be explicit when calling 'reset()' on a pointee of a smart pointer + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: use dereference to call 'reset' method of the pointee + // 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: be explicit when calling 'reset()' on a smart pointer with a pointee that has a 'reset()' method + // CHECK-MESSAGES: :[[@LINE-2]]:5: note: assign the pointer to 'nullptr' + // CHECK-FIXES: u_ptr = nullptr; + u_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: be explicit when calling 'reset()' on a pointee of a smart pointer + // CHECK-MESSAGES: :[[@LINE-2]]:5: note: use dereference to call 'reset' method of the pointee + // CHECK-FIXES: (*u_ptr).reset(); + u_ptr.reset(nullptr); + u_ptr->doSomething(); + + s_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: be explicit when calling 'reset()' on a smart pointer with a pointee that has a 'reset()' method + // CHECK-MESSAGES: :[[@LINE-2]]:5: note: assign the pointer to 'nullptr' + // CHECK-FIXES: s_ptr = nullptr; + s_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: be explicit when calling 'reset()' on a pointee of a smart pointer + // CHECK-MESSAGES: :[[@LINE-2]]:5: note: use dereference to call 'reset' method of the pointee + // 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 851afcc804ad3c55e3bd837e6dadeb8166c4c6ad Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Fri, 31 Jan 2025 19:33:02 +0300 Subject: [PATCH 2/2] [clang-tidy] add newline in test file --- .../bugprone/smartptr-reset-ambiguous-call-custom-pointers.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index bcb2498bd2043e..2b8944c39619d2 100644 --- 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 @@ -69,4 +69,4 @@ void Negative() { 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