Detect and fail if using mismatched holders by rhaschke · Pull Request #2644 · pybind/pybind11 (original) (raw)

It's 8bits vs 32bits actually wink

A bool, yes, but this is a bitfield : 1. And one that still fits in the alignment, so in practice probably 0 instead of 32, to be pedantic ;-)

Aren't holder types always templated? But, I understand your hesitation.

Yes, probably, but if you insist, I can make sure I set up some kind of holder where that doesn't work. But you're right it wouldn't be a frequent scenario, and maybe not even officially supported (the docs don't really mention it as possible or not-possible) ;-

Note that the string-based comparison is only required for the declaration of derived classes. Compatibility check for (the much more frequent) function signatures is based on pointers:
https://github.com/pybind/pybind11/pull/2644/files#diff-b8e97eea5f84a84248db1bc72afaccfd8ba28cad023917d52bdc6bbc3669cd74R1627)

Hmmm, does that mean I can't return/pass a std::unique_ptr<Base> instead of Derived? Oh, but that's already not possible anyway, if Base is not registered, I believe.

So yes (and actually, same_type can also do a string comparison if it needs to, but it's not trying to match substrings).
So agreeing that performance is not a huge issue, here. I'm just (maybe unnecessarily) scared of string manipulation when it comes to comparing types :-)

Good idea! A remaining open issue (with both approaches) might be extra optional template arguments, e.g. the deleter of unique_ptr. Usually, we don't want to allow changing the deleter from a base to an derived type. But there is one counterexample in the unit tests:

// Object derived but with public destructor and no Deleter in default holder
class MyObject4b : public MyObject4a {

Given your previous rebuttal on the string comparison, we'd need to test if this is worth the extra interface, though?

And yes, the deleter is hairy. I'm not sure what to think of that. Would this be something to explicitly require users to opt-in to (saying, "yes, it's fine that these holder types are different, I take responsibility") ?
It's also another instance where substringing unique_ptr<A, FooDeleter>/unique_ptr<A, BarDeleter> to unique_ptr (cfr. the previous point) is tricky, though?