Remove objects from the inactive overload cache when they are deregistered

Add TODOs

Author: BetsyMcPhail

include/pybind11/detail/class.h

@@ -225,6 +225,18 @@ inline bool deregister_instance_impl(void *ptr, instance *self) {
     for (auto it = range.first; it != range.second; ++it) {
         if (self == it->second && Py_TYPE(self) == Py_TYPE(it->second)) {
             registered_instances.erase(it);
+            // TODO(eric.cousineau): This section can be removed if we can use
+            // a different key, as mentioned in `get_type_overload`.
+            PyObject *to_remove = (PyObject *)it->second;
+            auto &cache = detail::get_internals().inactive_overload_cache;
+            for (auto key = cache.begin(); key != cache.end();) {
+               if (key->first == to_remove) {
+                  key = cache.erase(key);
+               }
+               else {
+                  ++key;
+               }
+            }
             return true;
         }
     }

include/pybind11/pybind11.h

@@ -2410,8 +2410,15 @@ inline function get_type_overload(const void *this_ptr, const detail::type_info
     handle self = detail::get_object_handle(this_ptr, this_type);
     if (!self)
         return function();
-    handle type = self.get_type();
-    auto key = std::make_pair(type.ptr(), name);
+    // N.B. Rather than use `type.ptr()` (which upstream uses), use
+    // `self.ptr()` for better uniqueness to avoid drake#11424.
+    // TODO(eric.cousineau): Consider using something like the tuple
+    // (cls, f"{cls.__module__}.{cls.__qualname__}", name). However, since
+    // internals::inactive_overload_cache only supports
+    // `(PyObject*, const char*)` (and thus avoids memory management), it may
+    // require some hand-crafting and really bumping the internals version. For
+    // now, leave as-is.
+    auto key = std::make_pair(self.ptr(), name);
 
     /* Cache functions that aren't overloaded in Python to avoid
        many costly Python dictionary lookups below */