From 2ab5c431728785abc3332df9ebc857bdf0f18123 Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Fri, 27 May 2022 00:46:19 -0700
Subject: [PATCH 01/12] Add test_perf_accessors (to be merged into
 test_pytypes).

---
 include/pybind11/pytypes.h    | 19 +++++++++++++
 tests/CMakeLists.txt          |  1 +
 tests/test_perf_accessors.cpp | 44 ++++++++++++++++++++++++++++++
 tests/test_perf_accessors.py  | 51 +++++++++++++++++++++++++++++++++++
 4 files changed, 115 insertions(+)
 create mode 100644 tests/test_perf_accessors.cpp
 create mode 100644 tests/test_perf_accessors.py

diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h
index c71f1b2e90..bcfce9987d 100644
--- a/include/pybind11/pytypes.h
+++ b/include/pybind11/pytypes.h
@@ -180,6 +180,10 @@ class object_api : public pyobject_tag {
 
 PYBIND11_NAMESPACE_END(detail)
 
+#if !defined(PYBIND11_HANDLE_REF_DEBUG) && !defined(NDEBUG)
+#    define PYBIND11_HANDLE_REF_DEBUG
+#endif
+
 /** \rst
     Holds a reference to a Python object (no reference counting)
 
@@ -209,6 +213,9 @@ class handle : public detail::object_api<handle> {
         this function automatically. Returns a reference to itself.
     \endrst */
     const handle &inc_ref() const & {
+#ifdef PYBIND11_HANDLE_REF_DEBUG
+        inc_ref_counter(1);
+#endif
         Py_XINCREF(m_ptr);
         return *this;
     }
@@ -244,6 +251,18 @@ class handle : public detail::object_api<handle> {
 
 protected:
     PyObject *m_ptr = nullptr;
+
+#ifdef PYBIND11_HANDLE_REF_DEBUG
+private:
+    std::size_t static inc_ref_counter(std::size_t add) {
+        static std::size_t counter = 0;
+        counter += add;
+        return counter;
+    }
+
+public:
+    std::size_t static inc_ref_counter() { return inc_ref_counter(0); }
+#endif
 };
 
 /** \rst
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 7296cd1b81..4811fdf26b 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -145,6 +145,7 @@ set(PYBIND11_TEST_FILES
     test_numpy_vectorize
     test_opaque_types
     test_operator_overloading
+    test_perf_accessors
     test_pickling
     test_pytypes
     test_sequences_and_iterators
diff --git a/tests/test_perf_accessors.cpp b/tests/test_perf_accessors.cpp
new file mode 100644
index 0000000000..185a94feb7
--- /dev/null
+++ b/tests/test_perf_accessors.cpp
@@ -0,0 +1,44 @@
+#include "pybind11_tests.h"
+
+namespace test_perf_accessors {
+
+py::object perf_list_accessor(std::size_t num_iterations, int test_id) {
+    py::list l;
+    l.append(0);
+    auto var = l[0]; // Type of var is list accessor.
+    py::int_ answer(42);
+    var = answer; // Detach var from list, which releases a reference.
+#ifdef PYBIND11_HANDLE_REF_DEBUG
+    if (num_iterations == 0) {
+        py::list return_list;
+        std::size_t inc_refs = py::handle::inc_ref_counter();
+        var = answer;
+        inc_refs = py::handle::inc_ref_counter() - inc_refs;
+        return_list.append(inc_refs);
+        inc_refs = py::handle::inc_ref_counter();
+        var = 42;
+        inc_refs = py::handle::inc_ref_counter() - inc_refs;
+        return_list.append(inc_refs);
+        return return_list;
+    }
+#endif
+    if (test_id == 0) {
+        while (num_iterations != 0u) {
+            var = answer;
+            num_iterations--;
+        }
+    } else {
+        while (num_iterations != 0u) {
+            var = 42;
+            num_iterations--;
+        }
+    }
+    return py::none();
+}
+
+} // namespace test_perf_accessors
+
+TEST_SUBMODULE(perf_accessors, m) {
+    using namespace test_perf_accessors;
+    m.def("perf_list_accessor", perf_list_accessor);
+}
diff --git a/tests/test_perf_accessors.py b/tests/test_perf_accessors.py
new file mode 100644
index 0000000000..9beb3cb143
--- /dev/null
+++ b/tests/test_perf_accessors.py
@@ -0,0 +1,51 @@
+import time
+
+from pybind11_tests import perf_accessors as m
+
+
+def find_num_iterations(
+    callable,
+    callable_args,
+    num_iterations_first_pass,
+    num_iterations_target_elapsed_secs,
+    time_delta_floor=1.0e-6,
+    target_elapsed_secs_multiplier=1.05,  # Empirical.
+    target_elapsed_secs_tolerance=0.05,
+    search_max_iterations=100,
+):
+    td_target = num_iterations_target_elapsed_secs * target_elapsed_secs_multiplier
+    num_iterations = num_iterations_first_pass
+    for _ in range(search_max_iterations):
+        t0 = time.time()
+        callable(num_iterations, *callable_args)
+        td = time.time() - t0
+        num_iterations = max(
+            1, int(td_target * num_iterations / max(td, time_delta_floor))
+        )
+        if abs(td - td_target) / td_target < target_elapsed_secs_tolerance:
+            return num_iterations
+    raise RuntimeError("find_num_iterations failure: search_max_iterations exceeded.")
+
+
+def test_perf_list_accessor():
+    print(flush=True)
+    inc_refs = m.perf_list_accessor(0, 0)
+    print(f"{inc_refs=}", flush=True)
+    if inc_refs is not None:
+        assert inc_refs == [1, 1]
+    num_iterations = find_num_iterations(
+        callable=m.perf_list_accessor,
+        callable_args=(0,),
+        num_iterations_first_pass=100000,
+        num_iterations_target_elapsed_secs=2.0,
+    )
+    for test_id in range(2):
+        t0 = time.time()
+        m.perf_list_accessor(num_iterations, test_id)
+        secs = time.time() - t0
+        u_secs = secs * 10e6
+        per_call = u_secs / num_iterations
+        print(
+            f"PERF {test_id},{num_iterations},{secs:.3f}s,{per_call:.6f}μs",
+            flush=True,
+        )

From 64ea166059d28fa20cace22ba2ff785d735a9146 Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Fri, 27 May 2022 01:02:56 -0700
Subject: [PATCH 02/12] Python < 3.8 f-string compatibility

---
 tests/test_perf_accessors.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test_perf_accessors.py b/tests/test_perf_accessors.py
index 9beb3cb143..2d2d038a74 100644
--- a/tests/test_perf_accessors.py
+++ b/tests/test_perf_accessors.py
@@ -30,7 +30,7 @@ def find_num_iterations(
 def test_perf_list_accessor():
     print(flush=True)
     inc_refs = m.perf_list_accessor(0, 0)
-    print(f"{inc_refs=}", flush=True)
+    print(f"inc_refs={inc_refs}", flush=True)
     if inc_refs is not None:
         assert inc_refs == [1, 1]
     num_iterations = find_num_iterations(

From 7944d65d39bd487cc5590b6901681a07bae2bea8 Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Fri, 27 May 2022 16:00:21 -0700
Subject: [PATCH 03/12] Use thread_local in inc_ref_counter()

---
 include/pybind11/pytypes.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h
index bcfce9987d..4033524f2b 100644
--- a/include/pybind11/pytypes.h
+++ b/include/pybind11/pytypes.h
@@ -255,7 +255,7 @@ class handle : public detail::object_api<handle> {
 #ifdef PYBIND11_HANDLE_REF_DEBUG
 private:
     std::size_t static inc_ref_counter(std::size_t add) {
-        static std::size_t counter = 0;
+        thread_local std::size_t counter = 0;
         counter += add;
         return counter;
     }

From 383203818d5149314d5d1e46f64c920167c5990e Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Fri, 27 May 2022 16:48:21 -0700
Subject: [PATCH 04/12] Intentional breakage, brute-force way to quickly find
 out how many platforms reach the PYBIND11_HANDLE_REF_DEBUG code, with and
 without threads.

---
 include/pybind11/pytypes.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h
index 4033524f2b..38df816595 100644
--- a/include/pybind11/pytypes.h
+++ b/include/pybind11/pytypes.h
@@ -254,6 +254,11 @@ class handle : public detail::object_api<handle> {
 
 #ifdef PYBIND11_HANDLE_REF_DEBUG
 private:
+#if defined(WITH_THREAD)
+#error LOOOK_WITH_THREAD
+#else
+#error LOOOK_NOT_WITH_THREAD
+#endif
     std::size_t static inc_ref_counter(std::size_t add) {
         thread_local std::size_t counter = 0;
         counter += add;

From 2e08eccc431203969cf5881b0ad937dd3ebfef64 Mon Sep 17 00:00:00 2001
From: "pre-commit-ci[bot]"
 <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Date: Fri, 27 May 2022 23:49:05 +0000
Subject: [PATCH 05/12] [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci
---
 include/pybind11/pytypes.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h
index 38df816595..b2b3be03b7 100644
--- a/include/pybind11/pytypes.h
+++ b/include/pybind11/pytypes.h
@@ -254,11 +254,11 @@ class handle : public detail::object_api<handle> {
 
 #ifdef PYBIND11_HANDLE_REF_DEBUG
 private:
-#if defined(WITH_THREAD)
-#error LOOOK_WITH_THREAD
-#else
-#error LOOOK_NOT_WITH_THREAD
-#endif
+#    if defined(WITH_THREAD)
+#        error LOOOK_WITH_THREAD
+#    else
+#        error LOOOK_NOT_WITH_THREAD
+#    endif
     std::size_t static inc_ref_counter(std::size_t add) {
         thread_local std::size_t counter = 0;
         counter += add;

From 84ffecefc6cecefbae1c5b4e4a0d1911434ff96a Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Fri, 27 May 2022 23:03:58 -0700
Subject: [PATCH 06/12] Remove Intentional breakage

---
 include/pybind11/pytypes.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h
index b2b3be03b7..4033524f2b 100644
--- a/include/pybind11/pytypes.h
+++ b/include/pybind11/pytypes.h
@@ -254,11 +254,6 @@ class handle : public detail::object_api<handle> {
 
 #ifdef PYBIND11_HANDLE_REF_DEBUG
 private:
-#    if defined(WITH_THREAD)
-#        error LOOOK_WITH_THREAD
-#    else
-#        error LOOOK_NOT_WITH_THREAD
-#    endif
     std::size_t static inc_ref_counter(std::size_t add) {
         thread_local std::size_t counter = 0;
         counter += add;

From d82a44b888ff9fc3e6a0919e60342c7fb093d124 Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Fri, 27 May 2022 23:44:02 -0700
Subject: [PATCH 07/12] Drop perf test, move inc_refs tests to test_pytypes

---
 tests/CMakeLists.txt          |  1 -
 tests/test_perf_accessors.cpp | 44 ------------------------------
 tests/test_perf_accessors.py  | 51 -----------------------------------
 tests/test_pytypes.cpp        | 34 +++++++++++++++++++++++
 tests/test_pytypes.py         |  8 ++++++
 5 files changed, 42 insertions(+), 96 deletions(-)
 delete mode 100644 tests/test_perf_accessors.cpp
 delete mode 100644 tests/test_perf_accessors.py

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 4811fdf26b..7296cd1b81 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -145,7 +145,6 @@ set(PYBIND11_TEST_FILES
     test_numpy_vectorize
     test_opaque_types
     test_operator_overloading
-    test_perf_accessors
     test_pickling
     test_pytypes
     test_sequences_and_iterators
diff --git a/tests/test_perf_accessors.cpp b/tests/test_perf_accessors.cpp
deleted file mode 100644
index 185a94feb7..0000000000
--- a/tests/test_perf_accessors.cpp
+++ /dev/null
@@ -1,44 +0,0 @@
-#include "pybind11_tests.h"
-
-namespace test_perf_accessors {
-
-py::object perf_list_accessor(std::size_t num_iterations, int test_id) {
-    py::list l;
-    l.append(0);
-    auto var = l[0]; // Type of var is list accessor.
-    py::int_ answer(42);
-    var = answer; // Detach var from list, which releases a reference.
-#ifdef PYBIND11_HANDLE_REF_DEBUG
-    if (num_iterations == 0) {
-        py::list return_list;
-        std::size_t inc_refs = py::handle::inc_ref_counter();
-        var = answer;
-        inc_refs = py::handle::inc_ref_counter() - inc_refs;
-        return_list.append(inc_refs);
-        inc_refs = py::handle::inc_ref_counter();
-        var = 42;
-        inc_refs = py::handle::inc_ref_counter() - inc_refs;
-        return_list.append(inc_refs);
-        return return_list;
-    }
-#endif
-    if (test_id == 0) {
-        while (num_iterations != 0u) {
-            var = answer;
-            num_iterations--;
-        }
-    } else {
-        while (num_iterations != 0u) {
-            var = 42;
-            num_iterations--;
-        }
-    }
-    return py::none();
-}
-
-} // namespace test_perf_accessors
-
-TEST_SUBMODULE(perf_accessors, m) {
-    using namespace test_perf_accessors;
-    m.def("perf_list_accessor", perf_list_accessor);
-}
diff --git a/tests/test_perf_accessors.py b/tests/test_perf_accessors.py
deleted file mode 100644
index 2d2d038a74..0000000000
--- a/tests/test_perf_accessors.py
+++ /dev/null
@@ -1,51 +0,0 @@
-import time
-
-from pybind11_tests import perf_accessors as m
-
-
-def find_num_iterations(
-    callable,
-    callable_args,
-    num_iterations_first_pass,
-    num_iterations_target_elapsed_secs,
-    time_delta_floor=1.0e-6,
-    target_elapsed_secs_multiplier=1.05,  # Empirical.
-    target_elapsed_secs_tolerance=0.05,
-    search_max_iterations=100,
-):
-    td_target = num_iterations_target_elapsed_secs * target_elapsed_secs_multiplier
-    num_iterations = num_iterations_first_pass
-    for _ in range(search_max_iterations):
-        t0 = time.time()
-        callable(num_iterations, *callable_args)
-        td = time.time() - t0
-        num_iterations = max(
-            1, int(td_target * num_iterations / max(td, time_delta_floor))
-        )
-        if abs(td - td_target) / td_target < target_elapsed_secs_tolerance:
-            return num_iterations
-    raise RuntimeError("find_num_iterations failure: search_max_iterations exceeded.")
-
-
-def test_perf_list_accessor():
-    print(flush=True)
-    inc_refs = m.perf_list_accessor(0, 0)
-    print(f"inc_refs={inc_refs}", flush=True)
-    if inc_refs is not None:
-        assert inc_refs == [1, 1]
-    num_iterations = find_num_iterations(
-        callable=m.perf_list_accessor,
-        callable_args=(0,),
-        num_iterations_first_pass=100000,
-        num_iterations_target_elapsed_secs=2.0,
-    )
-    for test_id in range(2):
-        t0 = time.time()
-        m.perf_list_accessor(num_iterations, test_id)
-        secs = time.time() - t0
-        u_secs = secs * 10e6
-        per_call = u_secs / num_iterations
-        print(
-            f"PERF {test_id},{num_iterations},{secs:.3f}s,{per_call:.6f}μs",
-            flush=True,
-        )
diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp
index 8d296f655a..71db12cfb9 100644
--- a/tests/test_pytypes.cpp
+++ b/tests/test_pytypes.cpp
@@ -297,6 +297,40 @@ TEST_SUBMODULE(pytypes, m) {
         return d;
     });
 
+    m.def("accessor_moves", []() {
+        py::list return_list;
+#ifdef PYBIND11_HANDLE_REF_DEBUG
+        py::list lst;
+        lst.append(0);
+        auto list_accessor = lst[0];
+        auto tup = py::make_tuple(0);
+        auto tuple_accessor = tup[0];
+        py::int_ py_int_42(42);
+        // Detach accessors from containers, which releases references.
+        list_accessor = py_int_42;
+        tuple_accessor = py_int_42;
+
+        std::size_t inc_refs = py::handle::inc_ref_counter();
+        list_accessor = py_int_42; // l-value (to have as a control)
+        inc_refs = py::handle::inc_ref_counter() - inc_refs;
+        return_list.append(inc_refs);
+        inc_refs = py::handle::inc_ref_counter();
+        list_accessor = 42; // r-value
+        inc_refs = py::handle::inc_ref_counter() - inc_refs;
+        return_list.append(inc_refs);
+
+        inc_refs = py::handle::inc_ref_counter();
+        tuple_accessor = py_int_42; // l-value (to have as a control)
+        inc_refs = py::handle::inc_ref_counter() - inc_refs;
+        return_list.append(inc_refs);
+        inc_refs = py::handle::inc_ref_counter();
+        tuple_accessor = 42; // r-value
+        inc_refs = py::handle::inc_ref_counter() - inc_refs;
+        return_list.append(inc_refs);
+#endif
+        return return_list;
+    });
+
     // test_constructors
     m.def("default_constructors", []() {
         return py::dict("bytes"_a = py::bytes(),
diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py
index 9afe62f424..44b3632916 100644
--- a/tests/test_pytypes.py
+++ b/tests/test_pytypes.py
@@ -317,6 +317,14 @@ def func(self, x, *args):
     assert d["var"] == 99
 
 
+def test_accessor_moves():
+    inc_refs = m.accessor_moves()
+    if inc_refs:
+        assert inc_refs == [1, 1, 1, 1]
+    else:
+        pytest.skip("Not defined: PYBIND11_HANDLE_REF_DEBUG")
+
+
 def test_constructors():
     """C++ default and converting constructors are equivalent to type calls in Python"""
     types = [bytes, bytearray, str, bool, int, float, tuple, list, dict, set]

From f1752ffd9215a9b12f7520c7509c161d8a0471a0 Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Sat, 28 May 2022 12:56:51 -0700
Subject: [PATCH 08/12] Fold in PR #3970 with `#ifdef`s

---
 include/pybind11/pytypes.h | 58 ++++++++++++++++++++++++++++++++++++--
 tests/test_pytypes.cpp     |  5 ++++
 tests/test_pytypes.py      |  5 +++-
 3 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h
index 4033524f2b..09ff71beb5 100644
--- a/include/pybind11/pytypes.h
+++ b/include/pybind11/pytypes.h
@@ -85,7 +85,13 @@ class object_api : public pyobject_tag {
         or `object` subclass causes a call to ``__setitem__``.
     \endrst */
     item_accessor operator[](handle key) const;
-    /// See above (the only difference is that they key is provided as a string literal)
+#define PYBIND11_PR3970
+#ifdef PYBIND11_PR3970
+#    define PYBIND11_PR3970_ITEM_ACCESSOR
+    /// See above (the only difference is that the key's reference is stolen)
+    item_accessor operator[](object &&key) const;
+#endif
+    /// See above (the only difference is that the key is provided as a string literal)
     item_accessor operator[](const char *key) const;
 
     /** \rst
@@ -95,7 +101,12 @@ class object_api : public pyobject_tag {
         or `object` subclass causes a call to ``setattr``.
     \endrst */
     obj_attr_accessor attr(handle key) const;
-    /// See above (the only difference is that they key is provided as a string literal)
+    /// See above (the only difference is that the key's reference is stolen)
+#ifdef PYBIND11_PR3970
+#    define PYBIND11_PR3970_ATTR_ACCESSOR
+    obj_attr_accessor attr(object &&key) const;
+    /// See above (the only difference is that the key is provided as a string literal)
+#endif
     str_attr_accessor attr(const char *key) const;
 
     /** \rst
@@ -684,7 +695,12 @@ class accessor : public object_api<accessor<Policy>> {
     }
     template <typename T>
     void operator=(T &&value) & {
+#ifdef PYBIND11_PR3970
+#    define PYBIND11_PR3970_ENSURE_OBJECT
+        get_cache() = ensure_object(object_or_cast(std::forward<T>(value)));
+#else
         get_cache() = reinterpret_borrow<object>(object_or_cast(std::forward<T>(value)));
+#endif
     }
 
     template <typename T = Policy>
@@ -712,6 +728,11 @@ class accessor : public object_api<accessor<Policy>> {
     }
 
 private:
+#ifdef PYBIND11_PR3970_ENSURE_OBJECT
+    static object ensure_object(object &&o) { return std::move(o); }
+    static object ensure_object(handle h) { return reinterpret_borrow<object>(h); }
+#endif
+
     object &get_cache() const {
         if (!cache) {
             cache = Policy::get(obj, key);
@@ -1711,7 +1732,14 @@ class tuple : public object {
     size_t size() const { return (size_t) PyTuple_Size(m_ptr); }
     bool empty() const { return size() == 0; }
     detail::tuple_accessor operator[](size_t index) const { return {*this, index}; }
+#ifdef PYBIND11_PR3970
+    template <typename T, detail::enable_if_t<detail::is_pyobject<T>::value, int> = 0>
+    detail::item_accessor operator[](T o) const {
+        return object::operator[](std::forward<T>(o));
+    }
+#else
     detail::item_accessor operator[](handle h) const { return object::operator[](h); }
+#endif
     detail::tuple_iterator begin() const { return {*this, 0}; }
     detail::tuple_iterator end() const { return {*this, PyTuple_GET_SIZE(m_ptr)}; }
 };
@@ -1771,7 +1799,14 @@ class sequence : public object {
     }
     bool empty() const { return size() == 0; }
     detail::sequence_accessor operator[](size_t index) const { return {*this, index}; }
+#ifdef PYBIND11_PR3970
+    template <typename T, detail::enable_if_t<detail::is_pyobject<T>::value, int> = 0>
+    detail::item_accessor operator[](T o) const {
+        return object::operator[](std::forward<T>(o));
+    }
+#else
     detail::item_accessor operator[](handle h) const { return object::operator[](h); }
+#endif
     detail::sequence_iterator begin() const { return {*this, 0}; }
     detail::sequence_iterator end() const { return {*this, PySequence_Size(m_ptr)}; }
 };
@@ -1790,7 +1825,14 @@ class list : public object {
     size_t size() const { return (size_t) PyList_Size(m_ptr); }
     bool empty() const { return size() == 0; }
     detail::list_accessor operator[](size_t index) const { return {*this, index}; }
+#ifdef PYBIND11_PR3970
+    template <typename T, detail::enable_if_t<detail::is_pyobject<T>::value, int> = 0>
+    detail::item_accessor operator[](T o) const {
+        return object::operator[](std::forward<T>(o));
+    }
+#else
     detail::item_accessor operator[](handle h) const { return object::operator[](h); }
+#endif
     detail::list_iterator begin() const { return {*this, 0}; }
     detail::list_iterator end() const { return {*this, PyList_GET_SIZE(m_ptr)}; }
     template <typename T>
@@ -2085,6 +2127,12 @@ template <typename D>
 iterator object_api<D>::end() const {
     return iterator::sentinel();
 }
+#ifdef PYBIND11_PR3970_ITEM_ACCESSOR
+template <typename D>
+item_accessor object_api<D>::operator[](object &&key) const {
+    return {derived(), std::move(key)};
+}
+#endif
 template <typename D>
 item_accessor object_api<D>::operator[](handle key) const {
     return {derived(), reinterpret_borrow<object>(key)};
@@ -2093,6 +2141,12 @@ template <typename D>
 item_accessor object_api<D>::operator[](const char *key) const {
     return {derived(), pybind11::str(key)};
 }
+#ifdef PYBIND11_PR3970_ATTR_ACCESSOR
+template <typename D>
+obj_attr_accessor object_api<D>::attr(object &&key) const {
+    return {derived(), std::move(key)};
+}
+#endif
 template <typename D>
 obj_attr_accessor object_api<D>::attr(handle key) const {
     return {derived(), reinterpret_borrow<object>(key)};
diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp
index 71db12cfb9..e7c01cf056 100644
--- a/tests/test_pytypes.cpp
+++ b/tests/test_pytypes.cpp
@@ -300,6 +300,11 @@ TEST_SUBMODULE(pytypes, m) {
     m.def("accessor_moves", []() {
         py::list return_list;
 #ifdef PYBIND11_HANDLE_REF_DEBUG
+#    ifdef PYBIND11_PR3970
+        return_list.append("move");
+#    else
+        return_list.append("copy");
+#    endif
         py::list lst;
         lst.append(0);
         auto list_accessor = lst[0];
diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py
index 44b3632916..8a39e9c0be 100644
--- a/tests/test_pytypes.py
+++ b/tests/test_pytypes.py
@@ -320,7 +320,10 @@ def func(self, x, *args):
 def test_accessor_moves():
     inc_refs = m.accessor_moves()
     if inc_refs:
-        assert inc_refs == [1, 1, 1, 1]
+        if inc_refs[0] == "copy":
+            assert inc_refs == ["copy", 1, 1, 1, 1]
+        else:
+            assert inc_refs == ["move", 1, 0, 1, 0]
     else:
         pytest.skip("Not defined: PYBIND11_HANDLE_REF_DEBUG")
 

From b3470d0fb8f3bd39fdaed7826dd016c4ec5fda1f Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Sat, 28 May 2022 15:47:29 -0700
Subject: [PATCH 09/12] Complete test coverage for all newly added code.

---
 tests/test_pytypes.cpp | 74 ++++++++++++++++++++++++++++++++++--------
 tests/test_pytypes.py  |  6 ++--
 2 files changed, 64 insertions(+), 16 deletions(-)

diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp
index e7c01cf056..fbe94e87d9 100644
--- a/tests/test_pytypes.cpp
+++ b/tests/test_pytypes.cpp
@@ -301,35 +301,83 @@ TEST_SUBMODULE(pytypes, m) {
         py::list return_list;
 #ifdef PYBIND11_HANDLE_REF_DEBUG
 #    ifdef PYBIND11_PR3970
-        return_list.append("move");
+        return_list.append("+pr3970");
 #    else
-        return_list.append("copy");
+        return_list.append("-pr3970");
 #    endif
+        py::int_ py_int_0(0);
+        py::int_ py_int_42(42);
+        py::str py_str_count("count");
+
+        auto tup = py::make_tuple(0);
+
+        py::sequence seq(tup);
+
         py::list lst;
         lst.append(0);
-        auto list_accessor = lst[0];
-        auto tup = py::make_tuple(0);
-        auto tuple_accessor = tup[0];
-        py::int_ py_int_42(42);
-        // Detach accessors from containers, which releases references.
-        list_accessor = py_int_42;
-        tuple_accessor = py_int_42;
 
         std::size_t inc_refs = py::handle::inc_ref_counter();
-        list_accessor = py_int_42; // l-value (to have as a control)
+        tup[py_int_0]; // l-value (to have a control)
+        inc_refs = py::handle::inc_ref_counter() - inc_refs;
+        return_list.append(inc_refs);
+        inc_refs = py::handle::inc_ref_counter();
+        tup[py::int_(0)]; // r-value
+        inc_refs = py::handle::inc_ref_counter() - inc_refs;
+        return_list.append(inc_refs);
+
+        inc_refs = py::handle::inc_ref_counter();
+        tup.attr(py_str_count); // l-value (to have a control)
+        inc_refs = py::handle::inc_ref_counter() - inc_refs;
+        return_list.append(inc_refs);
+        inc_refs = py::handle::inc_ref_counter();
+        tup.attr(py::str("count")); // r-value
+        inc_refs = py::handle::inc_ref_counter() - inc_refs;
+        return_list.append(inc_refs);
+
+        inc_refs = py::handle::inc_ref_counter();
+        seq[py_int_0]; // l-value (to have a control)
+        inc_refs = py::handle::inc_ref_counter() - inc_refs;
+        return_list.append(inc_refs);
+        inc_refs = py::handle::inc_ref_counter();
+        seq[py::int_(0)]; // r-value
+        inc_refs = py::handle::inc_ref_counter() - inc_refs;
+        return_list.append(inc_refs);
+
+        inc_refs = py::handle::inc_ref_counter();
+        seq.attr(py_str_count); // l-value (to have a control)
+        inc_refs = py::handle::inc_ref_counter() - inc_refs;
+        return_list.append(inc_refs);
+        inc_refs = py::handle::inc_ref_counter();
+        seq.attr(py::str("count")); // r-value
+        inc_refs = py::handle::inc_ref_counter() - inc_refs;
+        return_list.append(inc_refs);
+
+        inc_refs = py::handle::inc_ref_counter();
+        lst[py_int_0]; // l-value (to have a control)
+        inc_refs = py::handle::inc_ref_counter() - inc_refs;
+        return_list.append(inc_refs);
+        inc_refs = py::handle::inc_ref_counter();
+        lst[py::int_(0)]; // r-value
+        inc_refs = py::handle::inc_ref_counter() - inc_refs;
+        return_list.append(inc_refs);
+
+        inc_refs = py::handle::inc_ref_counter();
+        lst.attr(py_str_count); // l-value (to have a control)
         inc_refs = py::handle::inc_ref_counter() - inc_refs;
         return_list.append(inc_refs);
         inc_refs = py::handle::inc_ref_counter();
-        list_accessor = 42; // r-value
+        lst.attr(py::str("count")); // r-value
         inc_refs = py::handle::inc_ref_counter() - inc_refs;
         return_list.append(inc_refs);
 
+        auto lst_acc = lst[py::int_(0)];
+        lst_acc = py::int_(42); // Detaches lst_acc from lst.
         inc_refs = py::handle::inc_ref_counter();
-        tuple_accessor = py_int_42; // l-value (to have as a control)
+        lst_acc = py_int_42; // l-value (to have a control)
         inc_refs = py::handle::inc_ref_counter() - inc_refs;
         return_list.append(inc_refs);
         inc_refs = py::handle::inc_ref_counter();
-        tuple_accessor = 42; // r-value
+        lst_acc = py::int_(42); // r-value
         inc_refs = py::handle::inc_ref_counter() - inc_refs;
         return_list.append(inc_refs);
 #endif
diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py
index 8a39e9c0be..979b391b29 100644
--- a/tests/test_pytypes.py
+++ b/tests/test_pytypes.py
@@ -320,10 +320,10 @@ def func(self, x, *args):
 def test_accessor_moves():
     inc_refs = m.accessor_moves()
     if inc_refs:
-        if inc_refs[0] == "copy":
-            assert inc_refs == ["copy", 1, 1, 1, 1]
+        if inc_refs[0].startswith("-"):
+            assert inc_refs == ["-pr3970", 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]
         else:
-            assert inc_refs == ["move", 1, 0, 1, 0]
+            assert inc_refs == ["+pr3970", 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0]
     else:
         pytest.skip("Not defined: PYBIND11_HANDLE_REF_DEBUG")
 

From ed1320cef0f602640382fe163fa5c5b18f0d12d7 Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Sat, 28 May 2022 16:07:30 -0700
Subject: [PATCH 10/12] Condense new unit tests via a simple local helper
 macro.

---
 tests/test_pytypes.cpp | 93 ++++++++++++++----------------------------
 1 file changed, 30 insertions(+), 63 deletions(-)

diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp
index fbe94e87d9..370d1ef3b3 100644
--- a/tests/test_pytypes.cpp
+++ b/tests/test_pytypes.cpp
@@ -297,7 +297,7 @@ TEST_SUBMODULE(pytypes, m) {
         return d;
     });
 
-    m.def("accessor_moves", []() {
+    m.def("accessor_moves", []() { // See PR #3970
         py::list return_list;
 #ifdef PYBIND11_HANDLE_REF_DEBUG
 #    ifdef PYBIND11_PR3970
@@ -316,70 +316,37 @@ TEST_SUBMODULE(pytypes, m) {
         py::list lst;
         lst.append(0);
 
-        std::size_t inc_refs = py::handle::inc_ref_counter();
-        tup[py_int_0]; // l-value (to have a control)
-        inc_refs = py::handle::inc_ref_counter() - inc_refs;
-        return_list.append(inc_refs);
-        inc_refs = py::handle::inc_ref_counter();
-        tup[py::int_(0)]; // r-value
-        inc_refs = py::handle::inc_ref_counter() - inc_refs;
-        return_list.append(inc_refs);
-
-        inc_refs = py::handle::inc_ref_counter();
-        tup.attr(py_str_count); // l-value (to have a control)
-        inc_refs = py::handle::inc_ref_counter() - inc_refs;
-        return_list.append(inc_refs);
-        inc_refs = py::handle::inc_ref_counter();
-        tup.attr(py::str("count")); // r-value
-        inc_refs = py::handle::inc_ref_counter() - inc_refs;
-        return_list.append(inc_refs);
-
-        inc_refs = py::handle::inc_ref_counter();
-        seq[py_int_0]; // l-value (to have a control)
-        inc_refs = py::handle::inc_ref_counter() - inc_refs;
-        return_list.append(inc_refs);
-        inc_refs = py::handle::inc_ref_counter();
-        seq[py::int_(0)]; // r-value
-        inc_refs = py::handle::inc_ref_counter() - inc_refs;
-        return_list.append(inc_refs);
-
-        inc_refs = py::handle::inc_ref_counter();
-        seq.attr(py_str_count); // l-value (to have a control)
-        inc_refs = py::handle::inc_ref_counter() - inc_refs;
-        return_list.append(inc_refs);
-        inc_refs = py::handle::inc_ref_counter();
-        seq.attr(py::str("count")); // r-value
-        inc_refs = py::handle::inc_ref_counter() - inc_refs;
-        return_list.append(inc_refs);
-
-        inc_refs = py::handle::inc_ref_counter();
-        lst[py_int_0]; // l-value (to have a control)
-        inc_refs = py::handle::inc_ref_counter() - inc_refs;
-        return_list.append(inc_refs);
-        inc_refs = py::handle::inc_ref_counter();
-        lst[py::int_(0)]; // r-value
-        inc_refs = py::handle::inc_ref_counter() - inc_refs;
-        return_list.append(inc_refs);
-
-        inc_refs = py::handle::inc_ref_counter();
-        lst.attr(py_str_count); // l-value (to have a control)
-        inc_refs = py::handle::inc_ref_counter() - inc_refs;
-        return_list.append(inc_refs);
-        inc_refs = py::handle::inc_ref_counter();
-        lst.attr(py::str("count")); // r-value
-        inc_refs = py::handle::inc_ref_counter() - inc_refs;
-        return_list.append(inc_refs);
+#    define PYBIND11_LOCAL_DEF(...)                                                               \
+        {                                                                                         \
+            std::size_t inc_refs = py::handle::inc_ref_counter();                                 \
+            __VA_ARGS__;                                                                          \
+            inc_refs = py::handle::inc_ref_counter() - inc_refs;                                  \
+            return_list.append(inc_refs);                                                         \
+        }
+
+        PYBIND11_LOCAL_DEF(tup[py_int_0])    // l-value (to have a control)
+        PYBIND11_LOCAL_DEF(tup[py::int_(0)]) // r-value
+
+        PYBIND11_LOCAL_DEF(tup.attr(py_str_count))     // l-value
+        PYBIND11_LOCAL_DEF(tup.attr(py::str("count"))) // r-value
+
+        PYBIND11_LOCAL_DEF(seq[py_int_0])    // l-value
+        PYBIND11_LOCAL_DEF(seq[py::int_(0)]) // r-value
+
+        PYBIND11_LOCAL_DEF(seq.attr(py_str_count))     // l-value
+        PYBIND11_LOCAL_DEF(seq.attr(py::str("count"))) // r-value
+
+        PYBIND11_LOCAL_DEF(lst[py_int_0])    // l-value
+        PYBIND11_LOCAL_DEF(lst[py::int_(0)]) // r-value
+
+        PYBIND11_LOCAL_DEF(lst.attr(py_str_count))     // l-value
+        PYBIND11_LOCAL_DEF(lst.attr(py::str("count"))) // r-value
 
         auto lst_acc = lst[py::int_(0)];
-        lst_acc = py::int_(42); // Detaches lst_acc from lst.
-        inc_refs = py::handle::inc_ref_counter();
-        lst_acc = py_int_42; // l-value (to have a control)
-        inc_refs = py::handle::inc_ref_counter() - inc_refs;
-        return_list.append(inc_refs);
-        inc_refs = py::handle::inc_ref_counter();
-        lst_acc = py::int_(42); // r-value
-        inc_refs = py::handle::inc_ref_counter() - inc_refs;
-        return_list.append(inc_refs);
+        lst_acc = py::int_(42);                    // Detaches lst_acc from lst.
+        PYBIND11_LOCAL_DEF(lst_acc = py_int_42)    // l-value
+        PYBIND11_LOCAL_DEF(lst_acc = py::int_(42)) // r-value
+#    undef PYBIND11_LOCAL_DEF
 #endif
         return return_list;
     });

From 0c63a12262c30568f65fb9645636dbeaacf5f943 Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Tue, 31 May 2022 12:21:42 -0700
Subject: [PATCH 11/12] Remove PYBIND11_PR3970 define. See
 https://github.com/pybind/pybind11/pull/3977#issuecomment-1142526417

---
 include/pybind11/pytypes.h | 58 ++------------------------------------
 tests/test_pytypes.cpp     |  5 ----
 tests/test_pytypes.py      |  6 ++--
 3 files changed, 4 insertions(+), 65 deletions(-)

diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h
index 09ff71beb5..4033524f2b 100644
--- a/include/pybind11/pytypes.h
+++ b/include/pybind11/pytypes.h
@@ -85,13 +85,7 @@ class object_api : public pyobject_tag {
         or `object` subclass causes a call to ``__setitem__``.
     \endrst */
     item_accessor operator[](handle key) const;
-#define PYBIND11_PR3970
-#ifdef PYBIND11_PR3970
-#    define PYBIND11_PR3970_ITEM_ACCESSOR
-    /// See above (the only difference is that the key's reference is stolen)
-    item_accessor operator[](object &&key) const;
-#endif
-    /// See above (the only difference is that the key is provided as a string literal)
+    /// See above (the only difference is that they key is provided as a string literal)
     item_accessor operator[](const char *key) const;
 
     /** \rst
@@ -101,12 +95,7 @@ class object_api : public pyobject_tag {
         or `object` subclass causes a call to ``setattr``.
     \endrst */
     obj_attr_accessor attr(handle key) const;
-    /// See above (the only difference is that the key's reference is stolen)
-#ifdef PYBIND11_PR3970
-#    define PYBIND11_PR3970_ATTR_ACCESSOR
-    obj_attr_accessor attr(object &&key) const;
-    /// See above (the only difference is that the key is provided as a string literal)
-#endif
+    /// See above (the only difference is that they key is provided as a string literal)
     str_attr_accessor attr(const char *key) const;
 
     /** \rst
@@ -695,12 +684,7 @@ class accessor : public object_api<accessor<Policy>> {
     }
     template <typename T>
     void operator=(T &&value) & {
-#ifdef PYBIND11_PR3970
-#    define PYBIND11_PR3970_ENSURE_OBJECT
-        get_cache() = ensure_object(object_or_cast(std::forward<T>(value)));
-#else
         get_cache() = reinterpret_borrow<object>(object_or_cast(std::forward<T>(value)));
-#endif
     }
 
     template <typename T = Policy>
@@ -728,11 +712,6 @@ class accessor : public object_api<accessor<Policy>> {
     }
 
 private:
-#ifdef PYBIND11_PR3970_ENSURE_OBJECT
-    static object ensure_object(object &&o) { return std::move(o); }
-    static object ensure_object(handle h) { return reinterpret_borrow<object>(h); }
-#endif
-
     object &get_cache() const {
         if (!cache) {
             cache = Policy::get(obj, key);
@@ -1732,14 +1711,7 @@ class tuple : public object {
     size_t size() const { return (size_t) PyTuple_Size(m_ptr); }
     bool empty() const { return size() == 0; }
     detail::tuple_accessor operator[](size_t index) const { return {*this, index}; }
-#ifdef PYBIND11_PR3970
-    template <typename T, detail::enable_if_t<detail::is_pyobject<T>::value, int> = 0>
-    detail::item_accessor operator[](T o) const {
-        return object::operator[](std::forward<T>(o));
-    }
-#else
     detail::item_accessor operator[](handle h) const { return object::operator[](h); }
-#endif
     detail::tuple_iterator begin() const { return {*this, 0}; }
     detail::tuple_iterator end() const { return {*this, PyTuple_GET_SIZE(m_ptr)}; }
 };
@@ -1799,14 +1771,7 @@ class sequence : public object {
     }
     bool empty() const { return size() == 0; }
     detail::sequence_accessor operator[](size_t index) const { return {*this, index}; }
-#ifdef PYBIND11_PR3970
-    template <typename T, detail::enable_if_t<detail::is_pyobject<T>::value, int> = 0>
-    detail::item_accessor operator[](T o) const {
-        return object::operator[](std::forward<T>(o));
-    }
-#else
     detail::item_accessor operator[](handle h) const { return object::operator[](h); }
-#endif
     detail::sequence_iterator begin() const { return {*this, 0}; }
     detail::sequence_iterator end() const { return {*this, PySequence_Size(m_ptr)}; }
 };
@@ -1825,14 +1790,7 @@ class list : public object {
     size_t size() const { return (size_t) PyList_Size(m_ptr); }
     bool empty() const { return size() == 0; }
     detail::list_accessor operator[](size_t index) const { return {*this, index}; }
-#ifdef PYBIND11_PR3970
-    template <typename T, detail::enable_if_t<detail::is_pyobject<T>::value, int> = 0>
-    detail::item_accessor operator[](T o) const {
-        return object::operator[](std::forward<T>(o));
-    }
-#else
     detail::item_accessor operator[](handle h) const { return object::operator[](h); }
-#endif
     detail::list_iterator begin() const { return {*this, 0}; }
     detail::list_iterator end() const { return {*this, PyList_GET_SIZE(m_ptr)}; }
     template <typename T>
@@ -2127,12 +2085,6 @@ template <typename D>
 iterator object_api<D>::end() const {
     return iterator::sentinel();
 }
-#ifdef PYBIND11_PR3970_ITEM_ACCESSOR
-template <typename D>
-item_accessor object_api<D>::operator[](object &&key) const {
-    return {derived(), std::move(key)};
-}
-#endif
 template <typename D>
 item_accessor object_api<D>::operator[](handle key) const {
     return {derived(), reinterpret_borrow<object>(key)};
@@ -2141,12 +2093,6 @@ template <typename D>
 item_accessor object_api<D>::operator[](const char *key) const {
     return {derived(), pybind11::str(key)};
 }
-#ifdef PYBIND11_PR3970_ATTR_ACCESSOR
-template <typename D>
-obj_attr_accessor object_api<D>::attr(object &&key) const {
-    return {derived(), std::move(key)};
-}
-#endif
 template <typename D>
 obj_attr_accessor object_api<D>::attr(handle key) const {
     return {derived(), reinterpret_borrow<object>(key)};
diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp
index 370d1ef3b3..ef214acc3b 100644
--- a/tests/test_pytypes.cpp
+++ b/tests/test_pytypes.cpp
@@ -300,11 +300,6 @@ TEST_SUBMODULE(pytypes, m) {
     m.def("accessor_moves", []() { // See PR #3970
         py::list return_list;
 #ifdef PYBIND11_HANDLE_REF_DEBUG
-#    ifdef PYBIND11_PR3970
-        return_list.append("+pr3970");
-#    else
-        return_list.append("-pr3970");
-#    endif
         py::int_ py_int_0(0);
         py::int_ py_int_42(42);
         py::str py_str_count("count");
diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py
index 979b391b29..b91a7e1564 100644
--- a/tests/test_pytypes.py
+++ b/tests/test_pytypes.py
@@ -320,10 +320,8 @@ def func(self, x, *args):
 def test_accessor_moves():
     inc_refs = m.accessor_moves()
     if inc_refs:
-        if inc_refs[0].startswith("-"):
-            assert inc_refs == ["-pr3970", 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]
-        else:
-            assert inc_refs == ["+pr3970", 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0]
+        # To be changed in PR #3970: [1, 0, 1, 0, ...]
+        assert inc_refs == [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]
     else:
         pytest.skip("Not defined: PYBIND11_HANDLE_REF_DEBUG")
 

From eae9d9895be80997da03e166db2fa88b93db678b Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Tue, 31 May 2022 12:26:50 -0700
Subject: [PATCH 12/12] Move static keyword first (fixes silly oversight).

---
 include/pybind11/pytypes.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h
index 4033524f2b..0c56900648 100644
--- a/include/pybind11/pytypes.h
+++ b/include/pybind11/pytypes.h
@@ -254,14 +254,14 @@ class handle : public detail::object_api<handle> {
 
 #ifdef PYBIND11_HANDLE_REF_DEBUG
 private:
-    std::size_t static inc_ref_counter(std::size_t add) {
+    static std::size_t inc_ref_counter(std::size_t add) {
         thread_local std::size_t counter = 0;
         counter += add;
         return counter;
     }
 
 public:
-    std::size_t static inc_ref_counter() { return inc_ref_counter(0); }
+    static std::size_t inc_ref_counter() { return inc_ref_counter(0); }
 #endif
 };