diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index cedb360f9e..75f6cfc24b 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -83,9 +83,9 @@ template using type_map = std::unordered_map; struct overload_hash { - inline size_t operator()(const std::pair& v) const { + inline size_t operator()(const std::pair& v) const { size_t value = std::hash()(v.first); - value ^= std::hash()(v.second) + 0x9e3779b9 + (value<<6) + (value>>2); + value ^= std::hash()(v.second); return value; } }; @@ -97,7 +97,7 @@ struct internals { type_map registered_types_cpp; // std::type_index -> pybind11's type information std::unordered_map> registered_types_py; // PyTypeObject* -> base type_info(s) std::unordered_multimap registered_instances; // void * -> instance* - std::unordered_set, overload_hash> inactive_overload_cache; + std::unordered_set, overload_hash> inactive_overload_cache; type_map> direct_conversions; std::unordered_map> patients; std::forward_list registered_exception_translators; diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index bc1104281f..61716ad820 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2426,7 +2426,10 @@ inline function get_type_overload(const void *this_ptr, const detail::type_info if (!self) return function(); handle type = self.get_type(); - auto key = std::make_pair(type.ptr(), name); + /* N.B. This uses `__qualname__.name` instead of `name` + to resolve pybind11#1922. */ + std::string full_name = type.attr("__qualname__").cast() + "." + name; + auto key = std::make_pair(type.ptr(), full_name); /* Cache functions that aren't overloaded in Python to avoid many costly Python dictionary lookups below */ diff --git a/tests/test_class.cpp b/tests/test_class.cpp index 499d0cc511..965f869e2a 100644 --- a/tests/test_class.cpp +++ b/tests/test_class.cpp @@ -367,6 +367,24 @@ TEST_SUBMODULE(class_, m) { .def(py::init<>()) .def("ptr", &Aligned::ptr); #endif + + // Test #1922 (drake#11424). + class ExampleVirt2 { + public: + virtual ~ExampleVirt2() {} + virtual std::string get_name() const { return "ExampleVirt2"; } + }; + class PyExampleVirt2 : public ExampleVirt2 { + public: + std::string get_name() const override { + PYBIND11_OVERLOAD(std::string, ExampleVirt2, get_name, ); + } + }; + py::class_(m, "ExampleVirt2") + .def(py::init()) + .def("get_name", &ExampleVirt2::get_name); + m.def("example_virt2_get_name", + [](const ExampleVirt2& obj) { return obj.get_name(); }); } template class BreaksBase { public: virtual ~BreaksBase() = default; }; diff --git a/tests/test_class.py b/tests/test_class.py index 0bdf65c4d9..21fe65e846 100644 --- a/tests/test_class.py +++ b/tests/test_class.py @@ -1,4 +1,5 @@ import pytest +import weakref from pybind11_tests import class_ as m from pybind11_tests import UserType, ConstructorStats @@ -289,3 +290,39 @@ def test_aligned(): if hasattr(m, "Aligned"): p = m.Aligned().ptr() assert p % 1024 == 0 + + +@pytest.mark.skip( + reason="Generally reproducible in CPython, Python 3, non-debug, on Linux. " + "However, hard to pin this down for CI.") +def test_1922(): + # Test #1922 (drake#11424). + # Define a derived class which *does not* overload the method. + # WARNING: The reproduction of this failure may be platform-specific, and + # seems to depend on the order of definition and/or the name of the classes + # defined. For example, trying to place this and the C++ code in + # `test_virtual_functions` makes `assert id_1 == id_2` below fail. + class Child1(m.ExampleVirt2): + pass + + id_1 = id(Child1) + assert m.example_virt2_get_name(m.ExampleVirt2()) == "ExampleVirt2" + assert m.example_virt2_get_name(Child1()) == "ExampleVirt2" + + # Now delete everything (and ensure it's deleted). + wref = weakref.ref(Child1) + del Child1 + pytest.gc_collect() + assert wref() is None + + # Define a derived class which *does* define an overload. + class Child2(m.ExampleVirt2): + def get_name(self): + return "Child2" + + id_2 = id(Child2) + assert id_1 == id_2 # This happens in CPython; not sure about PyPy. + assert m.example_virt2_get_name(m.ExampleVirt2()) == "ExampleVirt2" + # THIS WILL FAIL: This is using the cached `ExampleVirt2.get_name`, rather + # than re-inspect the Python dictionary. + assert m.example_virt2_get_name(Child2()) == "Child2"