-
Couldn't load subscription status.
- Fork 2.2k
type_caster_generic: add cast_sources abstraction #5866
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The refactor is awesome!
My comments are purely about naming, please feel free to disregard, but I believe giving a few minutes of thought to intuitive (less generic) naming could help future maintainers and potential new contributors a lot to navigate the pybind11 code more easily.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks!
|
It seems since the addition of this I am no longer able to compile. I get a It's entirely possible we're using pybind wrong of course, but it was compiling from a cursory glance, it seems I cannot have a something like this class declaration |
|
@PhilipDeegan I am unable to reproduce this compiler error using your example code. I somewhat doubt that you actually tested using your example code, because it doesn't compile (it's missing a semicolon after the definition of Please provide the entire compiler error. There should be quite a bit more after the two lines you pasted. There is probably some relevant content before those lines as well. What compiler version and operating system are you running on? Do you have any local customizations to pybind11 in your project? It would be very helpful if you can provide a full example (not a sketch) that you have confirmed reproduces the problem. |
|
@oremanj thanks for the reply our project is somewhat complex so I was trying to minimize the overhead
several, locally I have gcc 14 on debian 13 and our CI is clang 19 on Fedora 41, and both seem to show this full error log from our CI is here the stack in our code is from here the function call specifically is this and the return type is a vector of this
not sure what this means to add, I tried switching from |
This error appears to be from a different compiler than the error you pasted two lines of. Could you provide the rest of the error that you pasted two lines of? It seems to have more detail. Is there any way I could build your project locally? I see a github link so it's presumably open-source. Is the failing CI a Github Actions job that you could link to?
Are you using pybind11 completely unmodified as it exists in this repository, or do you have your own copy of it with some things changed?
That would surprise me, but this whole thing surprises me. Can you provide that error as well? |
|
I think I see the problem. You've created a class that is implicitly convertible from anything at all, including a pointer to itself. This PR added an implicit conversion in the call to From a C++ style perspective, this constructor should either be This is an easy fix in pybind also though. I'll put up a PR shortly. |
|
oh maybe, you're thinking this?
|
|
Yes. |
|
I confirm it works, cheers! |
|
Posted #5873 which should fix this on the pybind side. |
Description
This is a pure refactor (no observable behavior changes) that centralizes the logic for choosing the C++ source object and type to use for a to-Python conversion. It replaces the old
src_and_type()function (also written by yours truly, some eight years ago) that had some cumbersome interactions withsmart_holder. It anticipates changes that will be needed in #5800 to provide more information totype_caster_generic::cast()for it to use when casting to a foreign type; such information will be carried in thecast_sourcesobject. Withcast_sources, the downcasting and type lookup behavior can be separated from the call tocast(), so it becomes possible to insert custom logic before or after the actual cast, which is especially useful when casting to foreign via a holder.