wjakob / nanobind

nanobind: tiny and efficient C++/Python bindings
BSD 3-Clause "New" or "Revised" License
2.14k stars 161 forks source link

Add exception to assert introduced in #549 when the type caster of `U*` can produce the actual underlying object `U` #608

Closed francesco-ballarin closed 3 weeks ago

francesco-ballarin commented 3 weeks ago

Follow up to discussion #605.

I confirmed with my downstream project that the patch does indeed fix the regression introduced in nanobind 2.0. Please note that, even though the commit has my name and email address, the actual patch was provided by @oremanj.

wjakob commented 3 weeks ago

You need is_pointer_v, etc.

wjakob commented 3 weeks ago

What would be the best way to explain this change in the changelog @oremanj ?

francesco-ballarin commented 3 weeks ago

You need is_pointer_v, etc.

Hopefully after four attempts I've got it right :(

oremanj commented 3 weeks ago

I originally used conjunction because I was worried about potentially forming an invalid expression if T was not a pointer type, but on second thought I don’t think it helps or matters.

Possible changelog entry: “nanobind no longer prevents casting to a C++ container of pointers T* where T is a type with a user-defined type caster if the caster seems to operate by extracting a T* from the Python object rather than a T.” with a link to the issue that prompted this PR for more context.

I’m mostly offline until Monday and will respond to this in more depth at that time if needed.

francesco-ballarin commented 3 weeks ago

Thanks @oremanj , I've added what you suggested to the changelog.

wjakob commented 3 weeks ago

Merged, thanks!