From dd01665e5a8f5c13f18c009c34c1449c33b33dec Mon Sep 17 00:00:00 2001 From: Dean Moldovan Date: Thu, 16 Feb 2017 23:02:56 +0100 Subject: [PATCH] Enable static properties (py::metaclass) by default Now that only one shared metaclass is ever allocated, it's extremely cheap to enable it for all pybind11 types. * Deprecate the default py::metaclass() since it's not needed anymore. * Allow users to specify a custom metaclass via py::metaclass(handle). --- docs/advanced/classes.rst | 21 ++++++--------------- include/pybind11/attr.h | 21 ++++++++++++++------- include/pybind11/class_support.h | 14 ++++++-------- include/pybind11/pybind11.h | 12 ------------ tests/test_methods_and_attributes.cpp | 8 ++++++-- tests/test_methods_and_attributes.py | 16 ++++++++++++++++ tests/test_multiple_inheritance.cpp | 8 ++++---- tests/test_python_types.cpp | 2 +- 8 files changed, 53 insertions(+), 49 deletions(-) diff --git a/docs/advanced/classes.rst b/docs/advanced/classes.rst index e0463b451..232d13d59 100644 --- a/docs/advanced/classes.rst +++ b/docs/advanced/classes.rst @@ -437,24 +437,15 @@ The section on :ref:`properties` discussed the creation of instance properties that are implemented in terms of C++ getters and setters. Static properties can also be created in a similar way to expose getters and -setters of static class attributes. Two things are important to note: - -1. Static properties are implemented by instrumenting the *metaclass* of the - class in question -- however, this requires the class to have a modifiable - metaclass in the first place. pybind11 provides a ``py::metaclass()`` - annotation that must be specified in the ``class_`` constructor, or any - later method calls to ``def_{property_,∅}_{readwrite,readonly}_static`` will - fail (see the example below). - -2. For static properties defined in terms of setter and getter functions, note - that the implicit ``self`` argument also exists in this case and is used to - pass the Python ``type`` subclass instance. This parameter will often not be - needed by the C++ side, and the following example illustrates how to - instantiate a lambda getter function that ignores it: +setters of static class attributes. Note that the implicit ``self`` argument +also exists in this case and is used to pass the Python ``type`` subclass +instance. This parameter will often not be needed by the C++ side, and the +following example illustrates how to instantiate a lambda getter function +that ignores it: .. code-block:: cpp - py::class_(m, "Foo", py::metaclass()) + py::class_(m, "Foo") .def_property_readonly_static("foo", [](py::object /* self */) { return Foo(); }); Operator overloading diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index da3ac624f..7c3b84c81 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -54,7 +54,15 @@ struct dynamic_attr { }; struct buffer_protocol { }; /// Annotation which requests that a special metaclass is created for a type -struct metaclass { }; +struct metaclass { + handle value; + + PYBIND11_DEPRECATED("py::metaclass() is no longer required. It's turned on by default now.") + metaclass() = default; + + /// Override pybind11's default metaclass + explicit metaclass(handle value) : value(value) { } +}; /// Annotation to mark enums as an arithmetic type struct arithmetic { }; @@ -149,8 +157,7 @@ struct function_record { /// Special data structure which (temporarily) holds metadata about a bound class struct type_record { PYBIND11_NOINLINE type_record() - : multiple_inheritance(false), dynamic_attr(false), - buffer_protocol(false), metaclass(false) { } + : multiple_inheritance(false), dynamic_attr(false), buffer_protocol(false) { } /// Handle to the parent scope handle scope; @@ -179,6 +186,9 @@ struct type_record { /// Optional docstring const char *doc = nullptr; + /// Custom metaclass (optional) + handle metaclass; + /// Multiple inheritance marker bool multiple_inheritance : 1; @@ -188,9 +198,6 @@ struct type_record { /// Does the class implement the buffer protocol? bool buffer_protocol : 1; - /// Does the class require its own metaclass? - bool metaclass : 1; - /// Is the default (unique_ptr) holder type used? bool default_holder : 1; @@ -356,7 +363,7 @@ struct process_attribute : process_attribute_default struct process_attribute : process_attribute_default { - static void init(const metaclass &, type_record *r) { r->metaclass = true; } + static void init(const metaclass &m, type_record *r) { r->metaclass = m.value; } }; diff --git a/include/pybind11/class_support.h b/include/pybind11/class_support.h index 43ada0281..4cd81a177 100644 --- a/include/pybind11/class_support.h +++ b/include/pybind11/class_support.h @@ -244,7 +244,8 @@ inline PyObject *make_object_base_type(size_t instance_size) { issue no Python C API calls which could potentially invoke the garbage collector (the GC will call type_traverse(), which will in turn find the newly constructed type in an invalid state) */ - auto heap_type = (PyHeapTypeObject *) PyType_Type.tp_alloc(&PyType_Type, 0); + auto metaclass = get_internals().default_metaclass; + auto heap_type = (PyHeapTypeObject *) metaclass->tp_alloc(metaclass, 0); if (!heap_type) pybind11_fail("make_object_base_type(): error allocating type!"); @@ -437,7 +438,10 @@ inline PyObject* make_new_python_type(const type_record &rec) { issue no Python C API calls which could potentially invoke the garbage collector (the GC will call type_traverse(), which will in turn find the newly constructed type in an invalid state) */ - auto heap_type = (PyHeapTypeObject *) PyType_Type.tp_alloc(&PyType_Type, 0); + auto metaclass = rec.metaclass.ptr() ? (PyTypeObject *) rec.metaclass.ptr() + : internals.default_metaclass; + + auto heap_type = (PyHeapTypeObject *) metaclass->tp_alloc(metaclass, 0); if (!heap_type) pybind11_fail(std::string(rec.name) + ": Unable to create type object!"); @@ -457,12 +461,6 @@ inline PyObject* make_new_python_type(const type_record &rec) { /* Don't inherit base __init__ */ type->tp_init = pybind11_object_init; - /* Custom metaclass if requested (used for static properties) */ - if (rec.metaclass) { - Py_INCREF(internals.default_metaclass); - Py_TYPE(type) = (PyTypeObject *) internals.default_metaclass; - } - /* Supported protocols */ type->tp_as_number = &heap_type->as_number; type->tp_as_sequence = &heap_type->as_sequence; diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index ca6cebc73..cc468e48b 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -829,18 +829,6 @@ protected: const auto is_static = !(rec_fget->is_method && rec_fget->scope); const auto has_doc = rec_fget->doc && pybind11::options::show_user_defined_docstrings(); - if (is_static) { - auto mclass = handle((PyObject *) Py_TYPE(m_ptr)); - - if ((PyTypeObject *) mclass.ptr() == &PyType_Type) - pybind11_fail( - "Adding static properties to the type '" + - std::string(((PyTypeObject *) m_ptr)->tp_name) + - "' requires the type to have a custom metaclass. Please " - "ensure that one is created by supplying the pybind11::metaclass() " - "annotation to the associated class_<>(..) invocation."); - } - auto property = handle((PyObject *) (is_static ? get_internals().static_property_type : &PyProperty_Type)); attr(name) = property(fget.ptr() ? fget : none(), diff --git a/tests/test_methods_and_attributes.cpp b/tests/test_methods_and_attributes.cpp index 11c364039..b7b2edfd8 100644 --- a/tests/test_methods_and_attributes.cpp +++ b/tests/test_methods_and_attributes.cpp @@ -202,7 +202,7 @@ test_initializer methods_and_attributes([](py::module &m) { .def("__str__", &ExampleMandA::toString) .def_readwrite("value", &ExampleMandA::value); - py::class_(m, "TestProperties", py::metaclass()) + py::class_(m, "TestProperties") .def(py::init<>()) .def_readonly("def_readonly", &TestProperties::value) .def_readwrite("def_readwrite", &TestProperties::value) @@ -228,7 +228,7 @@ test_initializer methods_and_attributes([](py::module &m) { auto static_set2 = [](py::object, int v) { TestPropRVP::sv2.value = v; }; auto rvp_copy = py::return_value_policy::copy; - py::class_(m, "TestPropRVP", py::metaclass()) + py::class_(m, "TestPropRVP") .def(py::init<>()) .def_property_readonly("ro_ref", &TestPropRVP::get1) .def_property_readonly("ro_copy", &TestPropRVP::get2, rvp_copy) @@ -245,6 +245,10 @@ test_initializer methods_and_attributes([](py::module &m) { .def_property_readonly("rvalue", &TestPropRVP::get_rvalue) .def_property_readonly_static("static_rvalue", [](py::object) { return SimpleValue(); }); + struct MetaclassOverride { }; + py::class_(m, "MetaclassOverride", py::metaclass((PyObject *) &PyType_Type)) + .def_property_readonly_static("readonly", [](py::object) { return 1; }); + #if !defined(PYPY_VERSION) py::class_(m, "DynamicClass", py::dynamic_attr()) .def(py::init()); diff --git a/tests/test_methods_and_attributes.py b/tests/test_methods_and_attributes.py index b5d5afdcc..f185ac26d 100644 --- a/tests/test_methods_and_attributes.py +++ b/tests/test_methods_and_attributes.py @@ -126,6 +126,22 @@ def test_static_cls(): instance.static_cls = check_self +def test_metaclass_override(): + """Overriding pybind11's default metaclass changes the behavior of `static_property`""" + from pybind11_tests import MetaclassOverride + + assert type(ExampleMandA).__name__ == "pybind11_type" + assert type(MetaclassOverride).__name__ == "type" + + assert MetaclassOverride.readonly == 1 + assert type(MetaclassOverride.__dict__["readonly"]).__name__ == "pybind11_static_property" + + # Regular `type` replaces the property instead of calling `__set__()` + MetaclassOverride.readonly = 2 + assert MetaclassOverride.readonly == 2 + assert isinstance(MetaclassOverride.__dict__["readonly"], int) + + @pytest.mark.parametrize("access", ["ro", "rw", "static_ro", "static_rw"]) def test_property_return_value_policies(access): from pybind11_tests import TestPropRVP diff --git a/tests/test_multiple_inheritance.cpp b/tests/test_multiple_inheritance.cpp index a32f1da11..ef26cd849 100644 --- a/tests/test_multiple_inheritance.cpp +++ b/tests/test_multiple_inheritance.cpp @@ -123,24 +123,24 @@ test_initializer mi_static_properties([](py::module &pm) { .def(py::init<>()) .def("vanilla", &Vanilla::vanilla); - py::class_(m, "WithStatic1", py::metaclass()) + py::class_(m, "WithStatic1") .def(py::init<>()) .def_static("static_func1", &WithStatic1::static_func1) .def_readwrite_static("static_value1", &WithStatic1::static_value1); - py::class_(m, "WithStatic2", py::metaclass()) + py::class_(m, "WithStatic2") .def(py::init<>()) .def_static("static_func2", &WithStatic2::static_func2) .def_readwrite_static("static_value2", &WithStatic2::static_value2); py::class_( - m, "VanillaStaticMix1", py::metaclass()) + m, "VanillaStaticMix1") .def(py::init<>()) .def_static("static_func", &VanillaStaticMix1::static_func) .def_readwrite_static("static_value", &VanillaStaticMix1::static_value); py::class_( - m, "VanillaStaticMix2", py::metaclass()) + m, "VanillaStaticMix2") .def(py::init<>()) .def_static("static_func", &VanillaStaticMix2::static_func) .def_readwrite_static("static_value", &VanillaStaticMix2::static_value); diff --git a/tests/test_python_types.cpp b/tests/test_python_types.cpp index 99a3cb9ab..6a0044ca3 100644 --- a/tests/test_python_types.cpp +++ b/tests/test_python_types.cpp @@ -190,7 +190,7 @@ struct MoveOutContainer { test_initializer python_types([](py::module &m) { /* No constructor is explicitly defined below. An exception is raised when trying to construct it directly from Python */ - py::class_(m, "ExamplePythonTypes", "Example 2 documentation", py::metaclass()) + py::class_(m, "ExamplePythonTypes", "Example 2 documentation") .def("get_dict", &ExamplePythonTypes::get_dict, "Return a Python dictionary") .def("get_dict_2", &ExamplePythonTypes::get_dict_2, "Return a C++ dictionary") .def("get_list", &ExamplePythonTypes::get_list, "Return a Python list")