Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow specifying custom base classes #4247

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

virtuald
Copy link
Contributor

Description

In the GObject* (and others, such as QEMU's qdev) type system, C-compatible 'inheritance' is implemented by embedding the parent struct in the child struct:

struct Child {
    Parent object;
    ...
};

One can already implement a polymorphic_type_hook to go from Parent to Child objects, but currently you can't do:

py::class_<Child, Parent> c(m, "Child");

This allows specification of arbitrary base classes for a bound object, which allows this use case (and others as well).

Suggested changelog entry:

Base classes that don't follow normal C++ inheritance can now be specified using py::custom_base<T>

virtuald and others added 2 commits October 17, 2022 14:24
- Useful for object hierarchies that don't use C++ inheritance (such as GObject)
static void init(const custom_base<T> &b, type_record *r) {
r->add_base(typeid(T), b.fn);
// TODO: rename this to 'nonsimple'?
r->multiple_inheritance = true;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You really mean simple_ancestors=False, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't set that here, but that's what setting multiple_inheritance causes. See the TODO comment. :)

@rwgk
Copy link
Collaborator

rwgk commented Oct 18, 2022

First impression: amazing 1. how small the core changes are and 2. CI full success on first try.

I'm wondering about limitations and pitfalls, without having anything specific in mind, due to lack of familiarity with the internals you're touching. Can you think of any?

Some slightly random questions:

py::class_<MyException, py::custom_base<PyExc_Exception>(...)>
  • Do you have an example that needs the polymorphic_type_hook?

@virtuald
Copy link
Contributor Author

I expect that as I play with it I'll run into some pitfalls, but I have some other priorities to attend to before doing that. I suspect trampolines might not work, and if someone really wanted to bind GObject you'd need a custom metaclass.

Would this work with more than one custom base?

Definitely expect it to, since that's what the multiple-inheritance stuff is for.

py::class_<MyException, py::custom_base<PyExc_Exception>(...)>

Maybe. The limitation is that in py::custom_base<T>, I believe that T needs to be something that pybind11 has previously bound using py::class_, or at least py::cast needs to work? Though, honestly I'm not sure that's true either.

Do you have an example that needs the polymorphic_type_hook?

If you had a function that was of the form Base * get_base(Derived *), if I bind get_base and call it from python, I believe without a polymorphic type hook I believe that isinstance(get_base(d), Derived) == False. Haven't tried it yet either.

@virtuald
Copy link
Contributor Author

virtuald commented Nov 28, 2022

This weekend I played a bit with this concept. There's definitely a bit of work one needs to do to fully flesh this out. In particular, to support inheritance I needed to add an __init_subclass__ implementation (which depends on #4158) and an override for __init__ that did construction in a special way. It's still not fully working, but once that's done I should be able to put together a full example and some API extensions we can add here. Might be awhile though.

@Skylion007
Copy link
Collaborator

@virtuald Glad to find someone found a usecase for #4158, happy to merge that if it unblocks this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants