Details
-
Type:
Story
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: None
-
Labels:None
-
Story Points:4
-
Epic Link:
-
Sprint:DRP S17-2, DRP S17-3
-
Team:Data Release Production
Description
One frequently occurring problem with pybind11 is segfaults happening when two classes in an inheritance hierarchy have different holder types (i.e. parent has unique_ptr holder type while derived class has shared_ptr).
This ticket aims to investigate and implement (if possible) a compile time (or import time) check for matching holder types.
While this check is not strictly necessary, it would make the pybind11 stack significantly more robust.
Attachments
Activity
Looks good to me. Just a few minor comments:
- Perhaps the void_holder<std::unique_ptr<T>> specialization should have another template parameter for custom deleters?
- I thought I once read something somewhere about either shared_ptr<void> or unique_ptr<void> only being legal in C++14. I can't find that now, but could you please double-check that those are valid instantiations in C++11?
Jim Bosch. Good catch about the deleter, without that it fails to recognise a holder type that specified one.
I'm not sure about the problem with void. I can't find anything about that, and it compiles just fine with -std=c++11 without warnings. However, do note that creating a unique_ptr<void> is of course impossible without specifying a custom deleter. But asking for the typeid is valid (doesn't require a complete type when given a type instead of a reference).
Granted, this may be a bit of a hack, hence my expectation that the upstream developers may push back. But I can't think of another good way to encode the type information of the (smart) pointer that would not in itself be a hack. Of course I could just pick typeid(unique_ptr<int>) (or any other) and be done with it, but aesthetics pointed me to void.
Pim Schellart [X] (Inactive)
added a comment - Jim Bosch . Good catch about the deleter, without that it fails to recognise a holder type that specified one.
I'm not sure about the problem with void . I can't find anything about that, and it compiles just fine with -std=c++11 without warnings. However, do note that creating a unique_ptr<void> is of course impossible without specifying a custom deleter. But asking for the typeid is valid (doesn't require a complete type when given a type instead of a reference).
Granted, this may be a bit of a hack, hence my expectation that the upstream developers may push back. But I can't think of another good way to encode the type information of the (smart) pointer that would not in itself be a hack. Of course I could just pick typeid(unique_ptr<int>) (or any other) and be done with it, but aesthetics pointed me to void . If you didn't find any mention of shared_ptr<void> being disallowed, I probably imagined it. Carry on.
Merged upstream.
Pim Schellart [X] (Inactive)
added a comment - Merged upstream. 
Ok, Jim Bosch can you take a quick look at the PR?
but it is a thing to keep in mind with these kind of tickets.
I think upstream will probably think segfaults are a problem