[clang-tidy] Fix smart pointers handling in bugprone-use-after-move

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<const DeclRefExpr *> *DeclRefs);
 
  ASTContext *Context;
+ const bool AllowMovedSmartPtrUse;
  std::unique_ptr<ExprSequence> Sequence;
  std::unique_ptr<StmtToBlockMap> BlockMap;
  llvm::SmallPtrSet<const CFGBlock *, 8> 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<UseAfterMove>
 UseAfterMoveFinder::find(Stmt *CodeBlock, const Expr *MovingCall,
@@ -275,11 +277,13 @@ void UseAfterMoveFinder::getDeclRefs(
  DeclRefs](const ArrayRef<BoundNodes> Matches) {
  for (const auto &Match : Matches) {
  const auto *DeclRef = Match.getNodeAs<DeclRefExpr>("declref");
- const auto *Operator = Match.getNodeAs<CXXOperatorCallExpr>("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<CXXOperatorCallExpr>("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));

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

clang-tools-extra/docs/ReleaseNotes.rst

@@ -104,6 +104,11 @@ New check aliases
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+- Improved :doc:`bugprone-use-after-move
+ <clang-tidy/checks/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
 ^^^^^^^^^^^^^^
 

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<T []>``) is called on it.
+(in the case of ``std::unique_ptr<T []>``) 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`.
+

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<A>);
+};
+
+class ClassB {
+ ClassB(std::unique_ptr<A> aaa) : aa(std::move(aaa)) {
+ a = std::make_unique<SinkA>(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<A> aa;
+ std::unique_ptr<SinkA> a;
+};
+
+void s(const std::unique_ptr<A> &);
+
+template <typename T, typename... Args> auto my_make_unique(Args &&...args) {
+ return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
+}
+
+void natively(std::unique_ptr<A> x) {
+ std::unique_ptr<A> tmp = std::move(x);
+ std::unique_ptr<SinkA> 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<A> x) {
+ std::unique_ptr<A> tmp = std::move(x);
+ std::unique_ptr<SinkA> y2 =
+ std::make_unique<SinkA>(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<A> x) {
+ std::unique_ptr<A> tmp = std::move(x);
+ std::unique_ptr<SinkA> y2 = my_make_unique<SinkA>(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<A> x) {
+ std::unique_ptr<A> 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
+}
+
+}

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 <typename T>
-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 <typename T>
-struct shared_ptr {
- shared_ptr();
- T *get() const;
- explicit operator bool() const;
- void reset(T *ptr);
- T &operator*() const;
- T *operator->() const;
-};
-
 template <typename T>
 struct weak_ptr {
  weak_ptr();
@@ -89,41 +73,6 @@ DECLARE_STANDARD_CONTAINER(unordered_multimap);
 
 typedef basic_string<char> string;
 
-template <typename>
-struct remove_reference;
-
-template <typename _Tp>
-struct remove_reference {
- typedef _Tp type;
-};
-
-template <typename _Tp>
-struct remove_reference<_Tp &> {
- typedef _Tp type;
-};
-
-template <typename _Tp>
-struct remove_reference<_Tp &&> {
- typedef _Tp type;
-};
-
-template <typename _Tp>
-constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) noexcept {
- return static_cast<typename remove_reference<_Tp>::type &&>(__t);
-}
-
-template <class _Tp>
-constexpr _Tp&&
-forward(typename std::remove_reference<_Tp>::type& __t) noexcept {
- return static_cast<_Tp&&>(__t);
-}
-
-template <class _Tp>
-constexpr _Tp&&
-forward(typename std::remove_reference<_Tp>::type&& __t) noexcept {
- return static_cast<_Tp&&>(__t);
-}
-
 } // namespace std
 
 class A {

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;

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 <typename T>
  unique_ptr &operator=(unique_ptr<T> &&) { return *this; }
@@ -25,4 +28,43 @@ class unique_ptr {
  type *ptr;
 };
 
+template <typename>
+struct remove_reference;
+
+template <typename _Tp>
+struct remove_reference {
+ typedef _Tp type;
+};
+
+template <typename _Tp>
+struct remove_reference<_Tp &> {
+ typedef _Tp type;
+};
+
+template <typename _Tp>
+struct remove_reference<_Tp &&> {
+ typedef _Tp type;
+};
+
+template <typename _Tp>
+constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) noexcept {
+ return static_cast<typename remove_reference<_Tp>::type &&>(__t);
+}
+
+template <class _Tp>
+constexpr _Tp&&
+forward(typename std::remove_reference<_Tp>::type& __t) noexcept {
+ return static_cast<_Tp&&>(__t);
+}
+
+template <class _Tp>
+constexpr _Tp&&
+forward(typename std::remove_reference<_Tp>::type&& __t) noexcept {
+ return static_cast<_Tp&&>(__t);
+}
+
+template <typename T, typename... Args> std::unique_ptr<T> make_unique(Args &&...args) {
+ return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
+}
+
 } // namespace std