Replace all SMART_HOLDER_WIP comments with reminders, notes, or pointers. (#5336)

The SMART_HOLDER_WIP comments are mostly from 2021. In the meantime, the
smart_holder code was extremely thoroughly tested in the Google codebase
(production code). Additionally, testing via PyCLIF-pybind11 provided
thousands of diverse use cases that needed to satisfy many million unit tests
(the success rate was about 99.999%).

Author: rwgk

include/pybind11/detail/struct_smart_holder.h

@@ -215,7 +215,7 @@ struct smart_holder {
         // race conditions, but in the context of Python it is a bug (elsewhere)
         // if the Global Interpreter Lock (GIL) is not being held when this code
         // is reached.
-        // SMART_HOLDER_WIP: IMPROVABLE: assert(GIL is held).
+        // PYBIND11:REMINDER: This may need to be protected by a mutex in free-threaded Python.
         if (vptr.use_count() != 1) {
             throw std::invalid_argument(std::string("Cannot disown use_count != 1 (") + context
                                         + ").");
@@ -277,29 +277,32 @@ struct smart_holder {
         return hld;
     }
 
-    // Caller is responsible for ensuring preconditions (SMART_HOLDER_WIP: details).
+    // Caller is responsible for ensuring the complex preconditions
+    // (see `smart_holder_type_caster_support::load_helper`).
     void disown() {
         reset_vptr_deleter_armed_flag(false);
         is_disowned = true;
     }
 
-    // Caller is responsible for ensuring preconditions (SMART_HOLDER_WIP: details).
+    // Caller is responsible for ensuring the complex preconditions
+    // (see `smart_holder_type_caster_support::load_helper`).
     void reclaim_disowned() {
         reset_vptr_deleter_armed_flag(true);
         is_disowned = false;
     }
 
-    // Caller is responsible for ensuring preconditions (SMART_HOLDER_WIP: details).
+    // Caller is responsible for ensuring the complex preconditions
+    // (see `smart_holder_type_caster_support::load_helper`).
     void release_disowned() { vptr.reset(); }
 
-    // SMART_HOLDER_WIP: review this function.
     void ensure_can_release_ownership(const char *context = "ensure_can_release_ownership") const {
         ensure_is_not_disowned(context);
         ensure_vptr_is_using_builtin_delete(context);
         ensure_use_count_1(context);
     }
 
-    // Caller is responsible for ensuring preconditions (SMART_HOLDER_WIP: details).
+    // Caller is responsible for ensuring the complex preconditions
+    // (see `smart_holder_type_caster_support::load_helper`).
     void release_ownership() {
         reset_vptr_deleter_armed_flag(false);
         release_disowned();

include/pybind11/detail/type_caster_base.h

@@ -475,7 +475,7 @@ inline PyObject *make_new_instance(PyTypeObject *type);
 
 #ifdef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
 
-// SMART_HOLDER_WIP: Needs refactoring of existing pybind11 code.
+// PYBIND11:REMINDER: Needs refactoring of existing pybind11 code.
 inline bool deregister_instance(instance *self, void *valptr, const type_info *tinfo);
 
 PYBIND11_NAMESPACE_BEGIN(smart_holder_type_caster_support)
@@ -568,8 +568,7 @@ handle smart_holder_from_unique_ptr(std::unique_ptr<T, D> &&src,
 
     if (static_cast<void *>(src.get()) == src_raw_void_ptr) {
         // This is a multiple-inheritance situation that is incompatible with the current
-        // shared_from_this handling (see PR #3023).
-        // SMART_HOLDER_WIP: IMPROVABLE: Is there a better solution?
+        // shared_from_this handling (see PR #3023). Is there a better solution?
         src_raw_void_ptr = nullptr;
     }
     auto smhldr = smart_holder::from_unique_ptr(std::move(src), src_raw_void_ptr);
@@ -623,8 +622,8 @@ handle smart_holder_from_shared_ptr(const std::shared_ptr<T> &src,
     void *src_raw_void_ptr = static_cast<void *>(src_raw_ptr);
     const detail::type_info *tinfo = st.second;
     if (handle existing_inst = find_registered_python_instance(src_raw_void_ptr, tinfo)) {
-        // SMART_HOLDER_WIP: MISSING: Enforcement of consistency with existing smart_holder.
-        // SMART_HOLDER_WIP: MISSING: keep_alive.
+        // PYBIND11:REMINDER: MISSING: Enforcement of consistency with existing smart_holder.
+        // PYBIND11:REMINDER: MISSING: keep_alive.
         return existing_inst;
     }
 

include/pybind11/trampoline_self_life_support.h

@@ -15,14 +15,13 @@
 PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
 
 PYBIND11_NAMESPACE_BEGIN(detail)
-// SMART_HOLDER_WIP: Needs refactoring of existing pybind11 code.
+// PYBIND11:REMINDER: Needs refactoring of existing pybind11 code.
 inline bool deregister_instance(instance *self, void *valptr, const type_info *tinfo);
 PYBIND11_NAMESPACE_END(detail)
 
 // The original core idea for this struct goes back to PyCLIF:
 // https://github.com/google/clif/blob/07f95d7e69dca2fcf7022978a55ef3acff506c19/clif/python/runtime.cc#L37
-// URL provided here mainly to give proper credit. To fully explain the `HoldPyObj` feature, more
-// context is needed (SMART_HOLDER_WIP).
+// URL provided here mainly to give proper credit.
 struct trampoline_self_life_support {
     detail::value_and_holder v_h;
 

tests/test_class_sh_trampoline_shared_ptr_cpp_arg.py

@@ -62,7 +62,7 @@ def test_shared_ptr_arg_identity():
     del obj
     pytest.gc_collect()
 
-    # SMART_HOLDER_WIP: the behavior below is DIFFERENT from PR #2839
+    # NOTE: the behavior below is DIFFERENT from PR #2839
     # python reference is gone because it is not an Alias instance
     assert objref() is None
     assert tester.has_python_instance() is False