Commit 4b159230 by Jason Rhinelander

Made module_local types take precedence over global types

Attempting to mix py::module_local and non-module_local classes results
in some unexpected/undesirable behaviour:

- if a class is registered non-local by some other module, a later
  attempt to register it locally fails.  It doesn't need to: it is
  perfectly acceptable for the local registration to simply override
  the external global registration.
- going the other way (i.e. module `A` registers a type `T` locally,
  then `B` registers the same type `T` globally) causes a more serious
  issue: `A.T`'s constructors no longer work because the `self` argument
  gets converted to a `B.T`, which then fails to resolve.

Changing the cast precedence to prefer local over global fixes this and
makes it work more consistently, regardless of module load order.
parent 2640c950
...@@ -752,6 +752,11 @@ you will only be able to call the function with the local module's class: ...@@ -752,6 +752,11 @@ you will only be able to call the function with the local module's class:
Invoked with: <dogs.Dog object at 0x123> Invoked with: <dogs.Dog object at 0x123>
It is possible to use ``py::module_local()`` registrations in one module even if another module
registers the same type globally: within the module with the module-local definition, all C++
instances will be cast to the associated bound Python type. Outside the module, any such values
are converted to the global Python type created elsewhere.
.. note:: .. note::
STL bindings (as provided via the optional :file:`pybind11/stl_bind.h` STL bindings (as provided via the optional :file:`pybind11/stl_bind.h`
......
...@@ -266,17 +266,22 @@ PYBIND11_NOINLINE inline detail::type_info* get_type_info(PyTypeObject *type) { ...@@ -266,17 +266,22 @@ PYBIND11_NOINLINE inline detail::type_info* get_type_info(PyTypeObject *type) {
} }
/// Return the type info for a given C++ type; on lookup failure can either throw or return nullptr. /// Return the type info for a given C++ type; on lookup failure can either throw or return nullptr.
/// `check_global_types` can be specified as `false` to only check types registered locally to the
/// current module.
PYBIND11_NOINLINE inline detail::type_info *get_type_info(const std::type_index &tp, PYBIND11_NOINLINE inline detail::type_info *get_type_info(const std::type_index &tp,
bool throw_if_missing = false) { bool throw_if_missing = false,
bool check_global_types = true) {
std::type_index type_idx(tp); std::type_index type_idx(tp);
auto &types = get_internals().registered_types_cpp;
auto it = types.find(type_idx);
if (it != types.end())
return (detail::type_info *) it->second;
auto &locals = registered_local_types_cpp(); auto &locals = registered_local_types_cpp();
it = locals.find(type_idx); auto it = locals.find(type_idx);
if (it != locals.end()) if (it != locals.end())
return (detail::type_info *) it->second; return (detail::type_info *) it->second;
if (check_global_types) {
auto &types = get_internals().registered_types_cpp;
it = types.find(type_idx);
if (it != types.end())
return (detail::type_info *) it->second;
}
if (throw_if_missing) { if (throw_if_missing) {
std::string tname = tp.name(); std::string tname = tp.name();
detail::clean_type_id(tname); detail::clean_type_id(tname);
......
...@@ -822,7 +822,7 @@ protected: ...@@ -822,7 +822,7 @@ protected:
pybind11_fail("generic_type: cannot initialize type \"" + std::string(rec.name) + pybind11_fail("generic_type: cannot initialize type \"" + std::string(rec.name) +
"\": an object with that name is already defined"); "\": an object with that name is already defined");
if (get_type_info(*rec.type)) if (get_type_info(*rec.type, false /* don't throw */, !rec.module_local))
pybind11_fail("generic_type: type \"" + std::string(rec.name) + pybind11_fail("generic_type: type \"" + std::string(rec.name) +
"\" is already registered!"); "\" is already registered!");
......
...@@ -16,6 +16,10 @@ using NonLocalType = LocalBase<1>; ...@@ -16,6 +16,10 @@ using NonLocalType = LocalBase<1>;
using NonLocal2 = LocalBase<2>; using NonLocal2 = LocalBase<2>;
/// Tests within-module, different-compilation-unit local definition conflict: /// Tests within-module, different-compilation-unit local definition conflict:
using LocalExternal = LocalBase<3>; using LocalExternal = LocalBase<3>;
/// Mixed: registered local first, then global
using MixedLocalGlobal = LocalBase<4>;
/// Mixed: global first, then local (which fails)
using MixedGlobalLocal = LocalBase<5>;
// Simple bindings (used with the above): // Simple bindings (used with the above):
template <typename T, int Adjust, typename... Args> template <typename T, int Adjust, typename... Args>
......
...@@ -65,6 +65,18 @@ PYBIND11_MODULE(pybind11_cross_module_tests, m) { ...@@ -65,6 +65,18 @@ PYBIND11_MODULE(pybind11_cross_module_tests, m) {
py::bind_map<std::unordered_map<std::string, uint8_t>>(m, "NonLocalMap2", py::module_local(false)); py::bind_map<std::unordered_map<std::string, uint8_t>>(m, "NonLocalMap2", py::module_local(false));
}); });
// test_mixed_local_global
// We try this both with the global type registered first and vice versa (the order shouldn't
// matter).
m.def("register_mixed_global_local", [m]() {
bind_local<MixedGlobalLocal, 200>(m, "MixedGlobalLocal", py::module_local());
});
m.def("register_mixed_local_global", [m]() {
bind_local<MixedLocalGlobal, 2000>(m, "MixedLocalGlobal", py::module_local(false));
});
m.def("get_mixed_gl", [](int i) { return MixedGlobalLocal(i); });
m.def("get_mixed_lg", [](int i) { return MixedLocalGlobal(i); });
// test_internal_locals_differ // test_internal_locals_differ
m.def("local_cpp_types_addr", []() { return (uintptr_t) &py::detail::registered_local_types_cpp(); }); m.def("local_cpp_types_addr", []() { return (uintptr_t) &py::detail::registered_local_types_cpp(); });
} }
...@@ -57,6 +57,18 @@ TEST_SUBMODULE(local_bindings, m) { ...@@ -57,6 +57,18 @@ TEST_SUBMODULE(local_bindings, m) {
py::bind_vector<std::vector<NonLocal2>>(m, "LocalVec2", py::module_local()); py::bind_vector<std::vector<NonLocal2>>(m, "LocalVec2", py::module_local());
py::bind_map<std::unordered_map<std::string, uint8_t>>(m, "NonLocalMap2", py::module_local(false)); py::bind_map<std::unordered_map<std::string, uint8_t>>(m, "NonLocalMap2", py::module_local(false));
// test_mixed_local_global
// We try this both with the global type registered first and vice versa (the order shouldn't
// matter).
m.def("register_mixed_global", [m]() {
bind_local<MixedGlobalLocal, 100>(m, "MixedGlobalLocal", py::module_local(false));
});
m.def("register_mixed_local", [m]() {
bind_local<MixedLocalGlobal, 1000>(m, "MixedLocalGlobal", py::module_local());
});
m.def("get_mixed_gl", [](int i) { return MixedGlobalLocal(i); });
m.def("get_mixed_lg", [](int i) { return MixedLocalGlobal(i); });
// test_internal_locals_differ // test_internal_locals_differ
m.def("local_cpp_types_addr", []() { return (uintptr_t) &py::detail::registered_local_types_cpp(); }); m.def("local_cpp_types_addr", []() { return (uintptr_t) &py::detail::registered_local_types_cpp(); });
} }
...@@ -103,6 +103,37 @@ def test_stl_bind_global(): ...@@ -103,6 +103,37 @@ def test_stl_bind_global():
assert str(excinfo.value) == 'generic_type: type "NonLocalMap2" is already registered!' assert str(excinfo.value) == 'generic_type: type "NonLocalMap2" is already registered!'
def test_mixed_local_global():
"""Local types take precedence over globally registered types: a module with a `module_local`
type can be registered even if the type is already registered globally. With the module,
casting will go to the local type; outside the module casting goes to the global type."""
import pybind11_cross_module_tests as cm
m.register_mixed_global()
m.register_mixed_local()
a = []
a.append(m.MixedGlobalLocal(1))
a.append(m.MixedLocalGlobal(2))
a.append(m.get_mixed_gl(3))
a.append(m.get_mixed_lg(4))
assert [x.get() for x in a] == [101, 1002, 103, 1004]
cm.register_mixed_global_local()
cm.register_mixed_local_global()
a.append(m.MixedGlobalLocal(5))
a.append(m.MixedLocalGlobal(6))
a.append(cm.MixedGlobalLocal(7))
a.append(cm.MixedLocalGlobal(8))
a.append(m.get_mixed_gl(9))
a.append(m.get_mixed_lg(10))
a.append(cm.get_mixed_gl(11))
a.append(cm.get_mixed_lg(12))
assert [x.get() for x in a] == \
[101, 1002, 103, 1004, 105, 1006, 207, 2008, 109, 1010, 211, 2012]
def test_internal_locals_differ(): def test_internal_locals_differ():
"""Makes sure the internal local type map differs across the two modules""" """Makes sure the internal local type map differs across the two modules"""
import pybind11_cross_module_tests as cm import pybind11_cross_module_tests as cm
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment