https://github.com/vbvictor updated https://github.com/llvm/llvm-project/pull/121291
>From 099aacc263f70301efefa722d49874a47b0054a9 Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Sat, 1 Mar 2025 18:05:54 +0300 Subject: [PATCH] add ambigous-smartptr-reset-call check --- .../AmbiguousSmartptrResetCallCheck.cpp | 126 ++++++ .../AmbiguousSmartptrResetCallCheck.h | 37 ++ .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 + clang-tools-extra/docs/ReleaseNotes.rst | 6 + .../docs/clang-tidy/checks/list.rst | 1 + .../ambiguous-smartptr-reset-call.rst | 62 +++ ...us-smartptr-reset-call-custom-pointers.cpp | 52 +++ .../ambiguous-smartptr-reset-call.cpp | 397 ++++++++++++++++++ 9 files changed, 685 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/readability/AmbiguousSmartptrResetCallCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/AmbiguousSmartptrResetCallCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/ambiguous-smartptr-reset-call.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/ambiguous-smartptr-reset-call-custom-pointers.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/ambiguous-smartptr-reset-call.cpp diff --git a/clang-tools-extra/clang-tidy/readability/AmbiguousSmartptrResetCallCheck.cpp b/clang-tools-extra/clang-tidy/readability/AmbiguousSmartptrResetCallCheck.cpp new file mode 100644 index 0000000000000..5f36c3976fc69 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/AmbiguousSmartptrResetCallCheck.cpp @@ -0,0 +1,126 @@ +//===--- AmbiguousSmartptrResetCallCheck.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 "AmbiguousSmartptrResetCallCheck.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::readability { + +namespace { + +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;" + "::boost::shared_ptr"; +} // namespace + +AmbiguousSmartptrResetCallCheck::AmbiguousSmartptrResetCallCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + SmartPointers(utils::options::parseStringList( + Options.get("SmartPointers", DefaultSmartPointers))) {} + +void AmbiguousSmartptrResetCallCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "SmartPointers", + utils::options::serializeStringList(SmartPointers)); +} + +void AmbiguousSmartptrResetCallCheck::registerMatchers(MatchFinder *Finder) { + const auto IsSmartptr = hasAnyName(SmartPointers); + + const auto ResetMethod = + cxxMethodDecl(hasName("reset"), hasOnlyDefaultParameters()); + + const auto TypeWithReset = + anyOf(cxxRecordDecl( + anyOf(hasMethod(ResetMethod), + isDerivedFrom(cxxRecordDecl(hasMethod(ResetMethod))))), + classTemplateSpecializationDecl( + hasSpecializedTemplate(classTemplateDecl(has(ResetMethod))))); + + const auto SmartptrWithReset = expr(hasType(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(classTemplateSpecializationDecl( + IsSmartptr, + hasTemplateArgument( + 0, templateArgument(refersToType(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(TypeWithReset)))))))))))); + + Finder->addMatcher( + cxxMemberCallExpr( + callee(ResetMethod), + unless(hasAnyArgument(expr(unless(cxxDefaultArgExpr())))), + anyOf(on(cxxOperatorCallExpr(hasOverloadedOperatorName("->"), + hasArgument(0, SmartptrWithReset)) + .bind("ArrowOp")), + on(SmartptrWithReset))) + .bind("MemberCall"), + this); +} + +void AmbiguousSmartptrResetCallCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *MemberCall = + Result.Nodes.getNodeAs<CXXMemberCallExpr>("MemberCall"); + assert(MemberCall); + + if (const auto *Arrow = + Result.Nodes.getNodeAs<CXXOperatorCallExpr>("ArrowOp")) { + const CharSourceRange SmartptrSourceRange = + Lexer::getAsCharRange(Arrow->getArg(0)->getSourceRange(), + *Result.SourceManager, getLangOpts()); + + diag(MemberCall->getBeginLoc(), + "ambiguous call to 'reset()' on a pointee of a smart pointer, prefer " + "more explicit approach"); + + diag(MemberCall->getBeginLoc(), + "consider dereferencing smart pointer to call 'reset' method " + "of the pointee here", + DiagnosticIDs::Note) + << FixItHint::CreateInsertion(SmartptrSourceRange.getBegin(), "(*") + << FixItHint::CreateInsertion(SmartptrSourceRange.getEnd(), ")") + << FixItHint::CreateReplacement( + CharSourceRange::getCharRange( + Arrow->getOperatorLoc(), + Arrow->getOperatorLoc().getLocWithOffset(2)), + "."); + } else { + const auto *Member = cast<MemberExpr>(MemberCall->getCallee()); + assert(Member); + + diag(MemberCall->getBeginLoc(), + "ambiguous call to 'reset()' on a smart pointer with pointee that " + "also has a 'reset()' method, prefer more explicit approach"); + + diag(MemberCall->getBeginLoc(), + "consider assigning the pointer to 'nullptr' here", + DiagnosticIDs::Note) + << FixItHint::CreateReplacement( + SourceRange(Member->getOperatorLoc(), Member->getOperatorLoc()), + " =") + << FixItHint::CreateReplacement( + SourceRange(Member->getMemberLoc(), MemberCall->getEndLoc()), + " nullptr"); + } +} + +} // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/AmbiguousSmartptrResetCallCheck.h b/clang-tools-extra/clang-tidy/readability/AmbiguousSmartptrResetCallCheck.h new file mode 100644 index 0000000000000..05932e59e7928 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/AmbiguousSmartptrResetCallCheck.h @@ -0,0 +1,37 @@ +//===--- AmbiguousSmartptrResetCallCheck.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_READABILITY_AMBIGUOUSSMARTPTRRESETCALLCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AMBIGUOUSSMARTPTRRESETCALLCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::readability { + +/// 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/readability/ambiguous-smartptr-reset-call.html +class AmbiguousSmartptrResetCallCheck : public ClangTidyCheck { +public: + AmbiguousSmartptrResetCallCheck(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::readability + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AMBIGUOUSSMARTPTRRESETCALLCHECK_H diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 8f303c51e1b0d..4be1a8f831339 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS ) add_clang_library(clangTidyReadabilityModule STATIC + AmbiguousSmartptrResetCallCheck.cpp AvoidConstParamsInDecls.cpp AvoidNestedConditionalOperatorCheck.cpp AvoidReturnWithVoidValueCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index d61c0ba39658e..4c0812f0e6793 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -9,6 +9,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "AmbiguousSmartptrResetCallCheck.h" #include "AvoidConstParamsInDecls.h" #include "AvoidNestedConditionalOperatorCheck.h" #include "AvoidReturnWithVoidValueCheck.h" @@ -68,6 +69,8 @@ namespace readability { class ReadabilityModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck<AmbiguousSmartptrResetCallCheck>( + "readability-ambiguous-smartptr-reset-call"); CheckFactories.registerCheck<AvoidConstParamsInDecls>( "readability-avoid-const-params-in-decls"); CheckFactories.registerCheck<AvoidNestedConditionalOperatorCheck>( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 07a79d6bbe807..1cf882cb092d3 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -97,6 +97,12 @@ New checks Finds unintended character output from ``unsigned char`` and ``signed char`` to an ``ostream``. +- New :doc:`readability-ambiguous-smartptr-reset-call + <clang-tidy/checks/readability/ambiguous-smartptr-reset-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/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 5f03ef72cc603..c73bc8bff3539 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -353,6 +353,7 @@ Clang-Tidy Checks :doc:`portability-simd-intrinsics <portability/simd-intrinsics>`, :doc:`portability-std-allocator-const <portability/std-allocator-const>`, :doc:`portability-template-virtual-member-function <portability/template-virtual-member-function>`, + :doc:`readability-ambiguous-smartptr-reset-call <readability/ambiguous-smartptr-reset-call>`, "Yes" :doc:`readability-avoid-const-params-in-decls <readability/avoid-const-params-in-decls>`, "Yes" :doc:`readability-avoid-nested-conditional-operator <readability/avoid-nested-conditional-operator>`, :doc:`readability-avoid-return-with-void-value <readability/avoid-return-with-void-value>`, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/ambiguous-smartptr-reset-call.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/ambiguous-smartptr-reset-call.rst new file mode 100644 index 0000000000000..cf73839a46cfb --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/ambiguous-smartptr-reset-call.rst @@ -0,0 +1,62 @@ +.. title:: clang-tidy - readability-ambiguous-smartptr-reset-call + +readability-ambiguous-smartptr-reset-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 and classes that are considered are +``std::unique_ptr``, ``std::shared_ptr``, ``boost::shared_ptr``. To specify +other smart pointers or other classes use the :option:`SmartPointers` option. + + +.. note:: + + The check may emit invalid fix-its and misleading warning messages when + specifying custom smart pointers or other classes in the + :option:`SmartPointers` option. For example, ``boost::scoped_ptr`` does not + have an ``operator=`` which makes fix-its invalid. + +.. note:: + + Automatic fix-its are enabled only if :program:`clang-tidy` is invoked with + the `--fix-notes` option. + + +Options +------- + +.. option:: SmartPointers + + Semicolon-separated list of fully qualified class names of custom smart + pointers. Default value is `::std::unique_ptr;::std::shared_ptr; + ::boost::shared_ptr`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/ambiguous-smartptr-reset-call-custom-pointers.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/ambiguous-smartptr-reset-call-custom-pointers.cpp new file mode 100644 index 0000000000000..df3f16a9cf9ec --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/ambiguous-smartptr-reset-call-custom-pointers.cpp @@ -0,0 +1,52 @@ +// RUN: %check_clang_tidy %s readability-ambiguous-smartptr-reset-call %t -- \ +// RUN: -config='{CheckOptions: \ +// RUN: {readability-ambiguous-smartptr-reset-call.SmartPointers: "::std::unique_ptr;::other_ptr"}}' \ +// RUN: --fix-notes -- -I %S/../modernize/Inputs/smart-ptr + +#include "unique_ptr.h" +#include "shared_ptr.h" + +template <typename T> +struct other_ptr { + T& operator*() const; + T* operator->() const; + void reset(); +}; + +struct Resettable { + void reset(); + void doSomething(); +}; + +void Positive() { + std::unique_ptr<Resettable> u; + u.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: u = nullptr; + u->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // CHECK-FIXES: (*u).reset(); + + other_ptr<Resettable> s; + s.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: s = nullptr; + s->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // 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(); +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/ambiguous-smartptr-reset-call.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/ambiguous-smartptr-reset-call.cpp new file mode 100644 index 0000000000000..e6e7eb9231ec2 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/ambiguous-smartptr-reset-call.cpp @@ -0,0 +1,397 @@ +// RUN: %check_clang_tidy %s readability-ambiguous-smartptr-reset-call %t --fix-notes -- -I %S/../modernize/Inputs/smart-ptr + +#include "unique_ptr.h" +#include "shared_ptr.h" + +template <typename T> +struct non_default_reset_ptr { + T& operator*() const; + T* operator->() const; + void reset(T* p); +}; + +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: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: u = nullptr; + u->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // CHECK-FIXES: (*u).reset(); + + std::shared_ptr<Resettable> s; + s.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: s = nullptr; + s->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // CHECK-FIXES: (*s).reset(); + + std::unique_ptr<std::unique_ptr<int>> uu_ptr; + uu_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: uu_ptr = nullptr; + uu_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // CHECK-FIXES: (*uu_ptr).reset(); + + std::unique_ptr<std::shared_ptr<int>> su_ptr; + su_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: su_ptr = nullptr; + su_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // CHECK-FIXES: (*su_ptr).reset(); + + std::unique_ptr<ResettableWithDefaultParams> rd; + rd.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: rd = nullptr; + rd->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // 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: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: nested_ptr = nullptr; + nested_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // CHECK-FIXES: (*nested_ptr).reset(); + (*nested_ptr).reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: (*nested_ptr) = nullptr; + (*nested_ptr)->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // CHECK-FIXES: (*(*nested_ptr)).reset(); + (**nested_ptr).reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: (**nested_ptr) = nullptr; + (**nested_ptr)->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // 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); + + non_default_reset_ptr<Resettable> non_default_reset_ptr; + non_default_reset_ptr.reset(new Resettable); + non_default_reset_ptr->reset(); +} + +template <typename T> +void TemplatePositiveTest() { + std::unique_ptr<T> u_ptr; + + u_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: u_ptr = nullptr; + u_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // CHECK-FIXES: (*u_ptr).reset(); + + std::shared_ptr<T> s_ptr; + s_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: s_ptr = nullptr; + s_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // 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: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: u_ptr = nullptr; + u_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // CHECK-FIXES: (*u_ptr).reset(); + u_ptr.reset(nullptr); + u_ptr->doSomething(); + + s_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: s_ptr = nullptr; + s_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // 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; +}; + + +typedef std::unique_ptr<Resettable> TypedefResettableUniquePtr; +typedef std::shared_ptr<Resettable> TypedefResettableSharedPtr; + +void TypedefPositive() { + TypedefResettableUniquePtr u_ptr; + u_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: u_ptr = nullptr; + u_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // CHECK-FIXES: (*u_ptr).reset(); + + TypedefResettableSharedPtr s_ptr; + s_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: s_ptr = nullptr; + + s_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // CHECK-FIXES: (*s_ptr).reset(); +} + +using UsingResettableUniquePtr = std::unique_ptr<Resettable>; +using UsingResettableSharedPtr = std::shared_ptr<Resettable>; + +void UsingPositive() { + UsingResettableUniquePtr u_ptr; + u_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: u_ptr = nullptr; + u_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // CHECK-FIXES: (*u_ptr).reset(); + + UsingResettableSharedPtr s_ptr; + s_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: s_ptr = nullptr; + + s_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // CHECK-FIXES: (*s_ptr).reset(); +} + +template<typename T> +using UsingUniquePtr = std::unique_ptr<T>; +template<typename T> +using UsingSharedPtr = std::shared_ptr<T>; + +void UsingTemplatePositive() { + UsingUniquePtr<Resettable> u_ptr; + u_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: u_ptr = nullptr; + u_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // CHECK-FIXES: (*u_ptr).reset(); + + UsingSharedPtr<Resettable> s_ptr; + s_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: s_ptr = nullptr; + + s_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // CHECK-FIXES: (*s_ptr).reset(); +} + +template<typename T> +void UsingByTemplatePositive() { + UsingUniquePtr<T> u_ptr; + u_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: u_ptr = nullptr; + u_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // CHECK-FIXES: (*u_ptr).reset(); + + UsingSharedPtr<T> s_ptr; + s_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: s_ptr = nullptr; + + s_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // CHECK-FIXES: (*s_ptr).reset(); +} + +void instantiate2() { + UsingByTemplatePositive<Resettable>(); +} + +void NestedUsingPositive() { + UsingUniquePtr<UsingSharedPtr<TypedefResettableUniquePtr>> nested_ptr; + nested_ptr.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: nested_ptr = nullptr; + nested_ptr->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // CHECK-FIXES: (*nested_ptr).reset(); + (*nested_ptr).reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: (*nested_ptr) = nullptr; + (*nested_ptr)->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // CHECK-FIXES: (*(*nested_ptr)).reset(); + (**nested_ptr).reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: (**nested_ptr) = nullptr; + (**nested_ptr)->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // CHECK-FIXES: (*(**nested_ptr)).reset(); +} + +// Check other default pointers and classes. +namespace boost { + +template <typename T> +struct shared_ptr { + T& operator*() const; + T* operator->() const; + void reset(); + void reset(T*); +}; + +} // namespace boost + +void PositiveOtherClasses() { + boost::shared_ptr<Resettable> sh; + sh.reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a smart pointer with pointee that also has a 'reset()' method, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 'nullptr' here + // CHECK-FIXES: sh = nullptr; + sh->reset(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a pointee of a smart pointer, prefer more explicit approach + // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart pointer to call 'reset' method of the pointee here + // CHECK-FIXES: (*sh).reset(); +} + _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
