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

Add note about subclasses needing virtual functions #654

Merged
merged 2 commits into from Jul 23, 2017

Conversation

virtuald
Copy link
Contributor

@virtuald virtuald commented Feb 7, 2017

The wording for this is terrible, but I couldn't think of a clearer way to put it.

Fixes #645.

@jagerman
Copy link
Member

jagerman commented Feb 7, 2017

I think it might make more sense to have a sort of FAQ for new-to-C++ programmers section, which would include this as one of its items. I'm not sure whether it makes sense with just one question/answer, but perhaps there are other suitable bits of documentation that could be moved there as well?

@virtuald
Copy link
Contributor Author

virtuald commented Feb 7, 2017

If I were searching for why my object wasn't the right type, the first section I would go to is the 'inheritance' section.

@jagerman
Copy link
Member

I think the description could use some improvement. You actually need more than just a virtual function: that's part of it, but you also need to have a virtual destructor (or be inherited from a class with one), because destruction can happen via the base class pointer. And since a virtual destructor counts as a virtual function, typically that's all you need to worry about; other functions can be virtual as desired.

@dean0x7d
Copy link
Member

The wording here is very confusing. It definitely needs to change. But more fundamentally, I still don't think the docs should be peppered with explanations of introductory C++ concepts ("needs at least one virtual function" is literally the definition of a polymorphic class in C++).

I like @jagerman's suggestion for a specific section for new-to-C++ programmers (rather than having it spread around). Perhaps this would be best if someone who went the route of Python-to-C++ could list out some of the surprises and encounters with C++'s footguns.

@virtuald virtuald changed the title Add note about polymorphic types needing virtual functions Add note about subclasses needing virtual functions Mar 13, 2017
@virtuald
Copy link
Contributor Author

Yes, the wording sucks, and you're right, I didn't mean polymorphic types -- I really meant subclasses. I used to use C++ extensively, but haven't needed to use it in a few years, so I've forgotten some of the finer points such as this.

Here's a rewording:

When a wrapped function is used to return a base class pointer, that class must have at least one virtual function or the type of the object will not be inferred correctly.

At the end of the day, I don't care if it's in a FAQ or in that particular section, I would just like it to be somewhere so someone else doesn't spend 5 hours debugging something that they could have avoided by reading the documentation.

@jagerman
Copy link
Member

jagerman commented Mar 14, 2017

@virtuald - I think what @dean0x7d was getting at is that it's a bit hard for us to write such a FAQ, because we write C++ day-to-day. This point (about a virtual base) is a definite candidate for inclusion; were there other things you ran into that might be helpful, at least as a starting point? I'm not even asking you to write them up formally, but just to let us know what you think belongs in it (beyond this one).

@virtuald
Copy link
Contributor Author

I can't think of anything else at the moment. I would say that for the most part working with pybind11 has been a pleasure and far superior to other binding technologies I've used (SIP, SWIG, ctypes) in terms of ease of use, and I look forward to using it more in the future.

@dean0x7d
Copy link
Member

I think I misunderstood the original issue here. I see now that pybind11's automatic upcasting is not actually documented anywhere. I didn't really appreciate how significant and powerful that feature really is: we don't just get access to the base interface with polymorphic behavior, we actually get the concrete instance of the derived type complete with all methods and attributes (that the base may not even be aware of). This goes beyond what C++ polymorphism usually does.

I took a stab at documenting this. It should clarify how regular and polymorphic C++ types are exported to Python. Does this look OK to everyone?

@dean0x7d dean0x7d merged commit 7c0e2c2 into pybind:master Jul 23, 2017
@virtuald virtuald deleted the patch-2 branch December 30, 2018 07:22
@rwgk rwgk mentioned this pull request Feb 9, 2023
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.

None yet

3 participants