[clang-tools-extra] [clang-tidy] Add bugprone-smartptr-reset-ambiguous-call check (PR #121291)

[Denis Mikhailov via cfe-commits]

================
@@ -0,0 +1,48 @@
+.. 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 that are considered are ``std::unique_ptr``,
+``std::shared_ptr``, ``boost::unique_ptr``, ``boost::shared_ptr``. To specify
+other smart pointers or other classes use the :option:`SmartPointers` 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;
----------------
denzor200 wrote:

> consider adding std::optional here also (even that is not smart ptr)

Strictly disagree with you, optional-like classes are not assignable with `nullptr`.
Here is no place for them in this check otherwise it will produce wrong fix hints:

```
{
  std::optional<Resettable> o;
  o.reset(); // should produce `o = std::nullopt;` but `o = nullptr;` does
}
{
  boost::optional<Resettable> o;
  o.reset(); // should produce `o = boost::none;` but `o = nullptr;` does
}

```

https://godbolt.org/z/YM79G8zdP


https://github.com/llvm/llvm-project/pull/121291