diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp index 8f4b5e8092dda..e0f3b1227bce8 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp @@ -48,7 +48,7 @@ struct UseAfterMove { /// various internal helper functions). class UseAfterMoveFinder { public: - UseAfterMoveFinder(ASTContext *TheContext); + UseAfterMoveFinder(ASTContext *TheContext, bool AllowMovedSmartPtrUse); // Within the given code block, finds the first use of 'MovedVariable' that // occurs after 'MovingCall' (the expression that performs the move). If a @@ -71,6 +71,7 @@ class UseAfterMoveFinder { llvm::SmallPtrSetImpl *DeclRefs); ASTContext *Context; + const bool AllowMovedSmartPtrUse; std::unique_ptr Sequence; std::unique_ptr BlockMap; llvm::SmallPtrSet Visited; @@ -92,8 +93,9 @@ static StatementMatcher inDecltypeOrTemplateArg() { hasAncestor(expr(hasUnevaluatedContext()))); } -UseAfterMoveFinder::UseAfterMoveFinder(ASTContext *TheContext) - : Context(TheContext) {} +UseAfterMoveFinder::UseAfterMoveFinder(ASTContext *TheContext, + bool AllowMovedSmartPtrUse) + : Context(TheContext), AllowMovedSmartPtrUse(AllowMovedSmartPtrUse) {} std::optional UseAfterMoveFinder::find(Stmt *CodeBlock, const Expr *MovingCall, @@ -275,11 +277,13 @@ void UseAfterMoveFinder::getDeclRefs( DeclRefs](const ArrayRef Matches) { for (const auto &Match : Matches) { const auto *DeclRef = Match.getNodeAs("declref"); - const auto *Operator = Match.getNodeAs("operator"); if (DeclRef && BlockMap->blockContainingStmt(DeclRef) == Block) { // Ignore uses of a standard smart pointer that don't dereference the // pointer. - if (Operator || !isStandardSmartPointer(DeclRef->getDecl())) { + const auto *Operator = + Match.getNodeAs("operator"); + if (Operator || !AllowMovedSmartPtrUse || + !isStandardSmartPointer(DeclRef->getDecl())) { DeclRefs->insert(DeclRef); } } @@ -429,6 +433,13 @@ static void emitDiagnostic(const Expr *MovingCall, const DeclRefExpr *MoveArg, << IsMove; } } +UseAfterMoveCheck::UseAfterMoveCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + AllowMovedSmartPtrUse(Options.get("AllowMovedSmartPtrUse", false)) {} + +void UseAfterMoveCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AllowMovedSmartPtrUse", AllowMovedSmartPtrUse); +} void UseAfterMoveCheck::registerMatchers(MatchFinder *Finder) { // try_emplace is a common maybe-moving function that returns a @@ -520,7 +531,7 @@ void UseAfterMoveCheck::check(const MatchFinder::MatchResult &Result) { } for (Stmt *CodeBlock : CodeBlocks) { - UseAfterMoveFinder Finder(Result.Context); + UseAfterMoveFinder Finder(Result.Context, AllowMovedSmartPtrUse); if (auto Use = Finder.find(CodeBlock, MovingCall, Arg)) emitDiagnostic(MovingCall, Arg, *Use, this, Result.Context, determineMoveType(MoveDecl)); diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.h b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.h index c14e802847415..3fc660db1ff59 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.h @@ -20,13 +20,16 @@ namespace clang::tidy::bugprone { /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/use-after-move.html class UseAfterMoveCheck : public ClangTidyCheck { public: - UseAfterMoveCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + UseAfterMoveCheck(StringRef Name, ClangTidyContext *Context); bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus11; } void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + +private: + const bool AllowMovedSmartPtrUse; }; } // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 642ad39cc0c1c..263d4edf61506 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -104,6 +104,11 @@ New check aliases Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ +- Improved :doc:`bugprone-use-after-move + ` check to handle smart pointers + like any other objects allowing to detect more cases, previous behavior can + be restored by setting `AllowMovedSmartPtrUse` option to `true`. + Removed checks ^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst index 08bb5374bab1f..1cb39debce1a8 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst @@ -200,7 +200,8 @@ An exception to this are objects of type ``std::unique_ptr``, (objects of these classes are guaranteed to be empty after they have been moved from). Therefore, an object of these classes will only be considered to be used if it is dereferenced, i.e. if ``operator*``, ``operator->`` or ``operator[]`` -(in the case of ``std::unique_ptr``) is called on it. +(in the case of ``std::unique_ptr``) is called on it. This behavior can be +overridden with the option :option:`AllowMovedSmartPtrUse`. If multiple uses occur after a move, only the first of these is flagged. @@ -250,3 +251,14 @@ For example, if an additional member variable is added to ``S``, it is easy to forget to add the reinitialization for this additional member. Instead, it is safer to assign to the entire struct in one go, and this will also avoid the use-after-move warning. + +Options +------- + +.. option:: AllowMovedSmartPtrUse + + If this option is set to `true`, the check will not warn about uses of + ``std::unique_ptr``, ``std::shared_ptr`` that are not dereferences. This + can be useful if you are using these smart pointers in a way that is not + idiomatic, but that you know is safe. Default is `false`. + diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move-smart-ptr.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move-smart-ptr.cpp new file mode 100644 index 0000000000000..e62ba59768507 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move-smart-ptr.cpp @@ -0,0 +1,59 @@ +// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-use-after-move %t -- \ +// RUN: -config="{CheckOptions: {bugprone-use-after-move.AllowMovedSmartPtrUse: false}}" -- -fno-delayed-template-parsing -I %S/../modernize/Inputs/smart-ptr/ + +#include "unique_ptr.h" + +namespace PR90174 { + +struct A {}; + +struct SinkA { + SinkA(std::unique_ptr); +}; + +class ClassB { + ClassB(std::unique_ptr aaa) : aa(std::move(aaa)) { + a = std::make_unique(std::move(aaa)); + // CHECK-MESSAGES: [[@LINE-1]]:43: warning: 'aaa' used after it was moved + // CHECK-MESSAGES: [[@LINE-3]]:36: note: move occurred here + } + std::unique_ptr aa; + std::unique_ptr a; +}; + +void s(const std::unique_ptr &); + +template auto my_make_unique(Args &&...args) { + return std::unique_ptr(new T(std::forward(args)...)); +} + +void natively(std::unique_ptr x) { + std::unique_ptr tmp = std::move(x); + std::unique_ptr y2{new SinkA(std::move(x))}; + // CHECK-MESSAGES: [[@LINE-1]]:49: warning: 'x' used after it was moved + // CHECK-MESSAGES: [[@LINE-3]]:28: note: move occurred here +} + +void viaStdMakeUnique(std::unique_ptr x) { + std::unique_ptr tmp = std::move(x); + std::unique_ptr y2 = + std::make_unique(std::move(x)); + // CHECK-MESSAGES: [[@LINE-1]]:41: warning: 'x' used after it was moved + // CHECK-MESSAGES: [[@LINE-4]]:28: note: move occurred here +} + +void viaMyMakeUnique(std::unique_ptr x) { + std::unique_ptr tmp = std::move(x); + std::unique_ptr y2 = my_make_unique(std::move(x)); + // CHECK-MESSAGES: [[@LINE-1]]:63: warning: 'x' used after it was moved + // CHECK-MESSAGES: [[@LINE-3]]:28: note: move occurred here +} + +void viaMyMakeUnique2(std::unique_ptr x) { + std::unique_ptr tmp = std::move(x); + s(x); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'x' used after it was moved + // CHECK-MESSAGES: [[@LINE-3]]:28: note: move occurred here +} + +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp index 6a4e3990e36dc..5424ed76a02ea 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp @@ -1,32 +1,16 @@ -// RUN: %check_clang_tidy -std=c++11 -check-suffixes=,CXX11 %s bugprone-use-after-move %t -- -- -fno-delayed-template-parsing -// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-use-after-move %t -- -- -fno-delayed-template-parsing +// RUN: %check_clang_tidy -std=c++11 -check-suffixes=,CXX11 %s bugprone-use-after-move %t -- \ +// RUN: -config="{CheckOptions: {bugprone-use-after-move.AllowMovedSmartPtrUse: true}}" -- -fno-delayed-template-parsing -I %S/../modernize/Inputs/smart-ptr/ +// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-use-after-move %t -- \ +// RUN: -config="{CheckOptions: {bugprone-use-after-move.AllowMovedSmartPtrUse: true}}" -- -fno-delayed-template-parsing -I %S/../modernize/Inputs/smart-ptr/ + +#include "unique_ptr.h" +#include "shared_ptr.h" typedef decltype(nullptr) nullptr_t; namespace std { typedef unsigned size_t; -template -struct unique_ptr { - unique_ptr(); - T *get() const; - explicit operator bool() const; - void reset(T *ptr); - T &operator*() const; - T *operator->() const; - T& operator[](size_t i) const; -}; - -template -struct shared_ptr { - shared_ptr(); - T *get() const; - explicit operator bool() const; - void reset(T *ptr); - T &operator*() const; - T *operator->() const; -}; - template struct weak_ptr { weak_ptr(); @@ -89,41 +73,6 @@ DECLARE_STANDARD_CONTAINER(unordered_multimap); typedef basic_string string; -template -struct remove_reference; - -template -struct remove_reference { - typedef _Tp type; -}; - -template -struct remove_reference<_Tp &> { - typedef _Tp type; -}; - -template -struct remove_reference<_Tp &&> { - typedef _Tp type; -}; - -template -constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) noexcept { - return static_cast::type &&>(__t); -} - -template -constexpr _Tp&& -forward(typename std::remove_reference<_Tp>::type& __t) noexcept { - return static_cast<_Tp&&>(__t); -} - -template -constexpr _Tp&& -forward(typename std::remove_reference<_Tp>::type&& __t) noexcept { - return static_cast<_Tp&&>(__t); -} - } // namespace std class A { diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/smart-ptr/shared_ptr.h b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/smart-ptr/shared_ptr.h index 337cb28228b09..09613c54fee2f 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/smart-ptr/shared_ptr.h +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/smart-ptr/shared_ptr.h @@ -9,9 +9,11 @@ class __shared_ptr { public: type &operator*() { return *ptr; } type *operator->() { return ptr; } + type *get() const; type *release(); void reset(); void reset(type *pt); + explicit operator bool() const; private: type *ptr; diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/smart-ptr/unique_ptr.h b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/smart-ptr/unique_ptr.h index 5dc9e02b637a2..2fca61141c53e 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/smart-ptr/unique_ptr.h +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/smart-ptr/unique_ptr.h @@ -14,9 +14,12 @@ class unique_ptr { type &operator*() { return *ptr; } type *operator->() { return ptr; } type *release() { return ptr; } + type *get() const; + type& operator[](unsigned i) const; void reset() {} void reset(type *pt) {} void reset(type pt) {} + explicit operator bool() const; unique_ptr &operator=(unique_ptr &&) { return *this; } template unique_ptr &operator=(unique_ptr &&) { return *this; } @@ -25,4 +28,43 @@ class unique_ptr { type *ptr; }; +template +struct remove_reference; + +template +struct remove_reference { + typedef _Tp type; +}; + +template +struct remove_reference<_Tp &> { + typedef _Tp type; +}; + +template +struct remove_reference<_Tp &&> { + typedef _Tp type; +}; + +template +constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) noexcept { + return static_cast::type &&>(__t); +} + +template +constexpr _Tp&& +forward(typename std::remove_reference<_Tp>::type& __t) noexcept { + return static_cast<_Tp&&>(__t); +} + +template +constexpr _Tp&& +forward(typename std::remove_reference<_Tp>::type&& __t) noexcept { + return static_cast<_Tp&&>(__t); +} + +template std::unique_ptr make_unique(Args &&...args) { + return std::unique_ptr(new T(std::forward(args)...)); +} + } // namespace std