https://github.com/PiotrZSL updated
https://github.com/llvm/llvm-project/pull/94869
>From ceeb4006d54b40a226a7a1f4f78f7f5f9d9dd7a1 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <m...@piotrzegar.pl>
Date: Tue, 16 Jul 2024 18:34:25 +0000
Subject: [PATCH] [clang-tidy] Fix smart pointers handling in
bugprone-use-after-move
Removed custom handling of smart pointers and
added option IgnoreNonDerefSmartPtrs to restore
previous behavior if needed.
---
.../clang-tidy/bugprone/UseAfterMoveCheck.cpp | 23 +++++--
.../clang-tidy/bugprone/UseAfterMoveCheck.h | 7 +-
clang-tools-extra/docs/ReleaseNotes.rst | 4 +-
.../checks/bugprone/use-after-move.rst | 14 +++-
.../bugprone/use-after-move-smart-ptr.cpp | 59 +++++++++++++++++
.../checkers/bugprone/use-after-move.cpp | 65 ++-----------------
.../modernize/Inputs/smart-ptr/shared_ptr.h | 2 +
.../modernize/Inputs/smart-ptr/unique_ptr.h | 42 ++++++++++++
8 files changed, 148 insertions(+), 68 deletions(-)
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move-smart-ptr.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
index 8f4b5e8092dda..954b6324e9674 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 IgnoreNonDerefSmartPtrs);
// 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 IgnoreNonDerefSmartPtrs;
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 IgnoreNonDerefSmartPtrs)
+ : Context(TheContext), IgnoreNonDerefSmartPtrs(IgnoreNonDerefSmartPtrs) {}
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 || !IgnoreNonDerefSmartPtrs ||
+ !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),
+ IgnoreNonDerefSmartPtrs(Options.get("IgnoreNonDerefSmartPtrs", false)) {}
+
+void UseAfterMoveCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "IgnoreNonDerefSmartPtrs", IgnoreNonDerefSmartPtrs);
+}
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, IgnoreNonDerefSmartPtrs);
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..ace97c58c1966 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 IgnoreNonDerefSmartPtrs;
};
} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 004811d2eca4f..00ed708b3db6e 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -301,7 +301,9 @@ Changes in existing checks
calls to ``std::forward``. Fixed sequencing of designated initializers. Fixed
sequencing of callees: In C++17 and later, the callee of a function is
guaranteed
to be sequenced before the arguments, so don't warn if the use happens in the
- callee and the move happens in one of the arguments.
+ callee and the move happens in one of the arguments. Smart pointers are now
+ handled like any other objects allowing to detect more cases, previous
behavior
+ can be restored by setting `IgnoreNonDerefSmartPtrs` option to `true`.
- Improved :doc:`cppcoreguidelines-avoid-non-const-global-variables
<clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables>` check
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..8fd6d634f7891 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<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:`IgnoreNonDerefSmartPtrs`.
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:: IgnoreNonDerefSmartPtrs
+
+ 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..db06d3f121d79
--- /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.IgnoreNonDerefSmartPtrs: 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
+}
+
+}
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..1363a959b7991 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.IgnoreNonDerefSmartPtrs: 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.IgnoreNonDerefSmartPtrs: 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 {
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 <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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
