[clang-tools-extra] 0b344b4 - Extend bugprone-use-after-move check to handle std::optional::reset() and std::any::reset() (#114255)

[via cfe-commits]

Author: higher-performance
Date: 2024-11-16T00:47:52+08:00
New Revision: 0b344b4feff5cd04d63db7b914d783fd941fbda0

URL: https://github.com/llvm/llvm-project/commit/0b344b4feff5cd04d63db7b914d783fd941fbda0
DIFF: https://github.com/llvm/llvm-project/commit/0b344b4feff5cd04d63db7b914d783fd941fbda0.diff

LOG: Extend bugprone-use-after-move check to handle std::optional::reset() and std::any::reset() (#114255)

These need to be handled similarly to the standard smart pointers; they
all have a `reset` method.

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst
    clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
index 8f4b5e8092ddaa..960133159dbbf5 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -315,9 +315,10 @@ void UseAfterMoveFinder::getReinits(
           "::std::unordered_map", "::std::unordered_multiset",
           "::std::unordered_multimap"))))));
 
-  auto StandardSmartPointerTypeMatcher = hasType(hasUnqualifiedDesugaredType(
-      recordType(hasDeclaration(cxxRecordDecl(hasAnyName(
-          "::std::unique_ptr", "::std::shared_ptr", "::std::weak_ptr"))))));
+  auto StandardResettableOwnerTypeMatcher = hasType(
+      hasUnqualifiedDesugaredType(recordType(hasDeclaration(cxxRecordDecl(
+          hasAnyName("::std::unique_ptr", "::std::shared_ptr",
+                     "::std::weak_ptr", "::std::optional", "::std::any"))))));
 
   // Matches 
diff erent types of reinitialization.
   auto ReinitMatcher =
@@ -340,7 +341,7 @@ void UseAfterMoveFinder::getReinits(
                    callee(cxxMethodDecl(hasAnyName("clear", "assign")))),
                // reset() on standard smart pointers.
                cxxMemberCallExpr(
-                   on(expr(DeclRefMatcher, StandardSmartPointerTypeMatcher)),
+                   on(expr(DeclRefMatcher, StandardResettableOwnerTypeMatcher)),
                    callee(cxxMethodDecl(hasName("reset")))),
                // Methods that have the [[clang::reinitializes]] attribute.
                cxxMemberCallExpr(

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 2a826fbc69ea12..f967dfabd1c940 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -194,6 +194,11 @@ Changes in existing checks
   <clang-tidy/checks/bugprone/unsafe-functions>` check to allow specifying
   additional functions to match.
 
+- Improved :doc:`bugprone-use-after-move
+  <clang-tidy/checks/bugprone/use-after-move>` to avoid triggering on
+  ``reset()`` calls on moved-from ``std::optional`` and ``std::any`` objects,
+  similarly to smart pointers.
+
 - Improved :doc:`cert-flp30-c <clang-tidy/checks/cert/flp30-c>` check to
   fix false positive that floating point variable is only used in increment
   expression.

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 08bb5374bab1f4..965fc2d3c29e24 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
@@ -196,11 +196,13 @@ Any occurrence of the moved variable that is not a reinitialization (see below)
 is considered to be a use.
 
 An exception to this are objects of type ``std::unique_ptr``,
-``std::shared_ptr`` and ``std::weak_ptr``, which have defined move behavior
-(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.
+``std::shared_ptr``, ``std::weak_ptr``, ``std::optional``, and ``std::any``.
+An exception to this are objects of type ``std::unique_ptr``,
+``std::shared_ptr``, ``std::weak_ptr``, ``std::optional``, and ``std::any``, which
+can be reinitialized via ``reset``. For smart pointers specifically, the
+moved-from objects have a well-defined state of being ``nullptr``s, and only
+``operator*``, ``operator->`` and ``operator[]`` are considered bad accesses as
+they would be dereferencing a ``nullptr``.
 
 If multiple uses occur after a move, only the first of these is flagged.
 
@@ -222,7 +224,8 @@ The check considers a variable to be reinitialized in the following cases:
     ``unordered_multimap``.
 
   - ``reset()`` is called on the variable and the variable is of type
-    ``std::unique_ptr``, ``std::shared_ptr`` or ``std::weak_ptr``.
+    ``std::unique_ptr``, ``std::shared_ptr``, ``std::weak_ptr``,
+    ``std::optional``, or ``std::any``.
 
   - A member function marked with the ``[[clang::reinitializes]]`` attribute is
     called on the variable.

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 6a4e3990e36dc5..87dfec4f68061f 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
@@ -33,6 +33,19 @@ struct weak_ptr {
   bool expired() const;
 };
 
+template <typename T>
+struct optional {
+  optional();
+  T& operator*();
+  const T& operator*() const;
+  void reset();
+};
+
+struct any {
+  any();
+  void reset();
+};
+
 template <typename T1, typename T2>
 struct pair {};
 
@@ -257,6 +270,14 @@ void standardSmartPtr() {
     // CHECK-NOTES: [[@LINE-1]]:5: warning: 'ptr' used after it was moved
     // CHECK-NOTES: [[@LINE-3]]:5: note: move occurred here
   }
+  {
+    std::optional<A> opt;
+    std::move(opt);
+    A val = *opt;
+    (void)val;
+    // CHECK-NOTES: [[@LINE-2]]:14: warning: 'opt' used after it was moved
+    // CHECK-NOTES: [[@LINE-4]]:5: note: move occurred here
+  }
   {
     // std::weak_ptr<> cannot be dereferenced directly, so we only check that
     // member functions may be called on it after a move.
@@ -994,10 +1015,10 @@ void standardContainerAssignIsReinit() {
   }
 }
 
-// Resetting the standard smart pointer types using reset() is treated as a
+// Resetting the standard smart owning types using reset() is treated as a
 // re-initialization. (We don't test std::weak_ptr<> because it can't be
 // dereferenced directly.)
-void standardSmartPointerResetIsReinit() {
+void resetIsReinit() {
   {
     std::unique_ptr<A> ptr;
     std::move(ptr);
@@ -1010,6 +1031,20 @@ void standardSmartPointerResetIsReinit() {
     ptr.reset(new A);
     *ptr;
   }
+  {
+    std::optional<A> opt;
+    std::move(opt);
+    opt.reset();
+    std::optional<A> opt2 = opt;
+    (void)opt2;
+  }
+  {
+    std::any a;
+    std::move(a);
+    a.reset();
+    std::any a2 = a;
+    (void)a2;
+  }
 }
 
 void reinitAnnotation() {