Add check for matching holder_type when inheriting

Author: Pim Schellart

include/pybind11/attr.h

@@ -185,7 +185,7 @@ struct type_record {
     /// Does the class require its own metaclass?
     bool metaclass : 1;
 
-    PYBIND11_NOINLINE void add_base(const std::type_info *base, void *(*caster)(void *)) {
+    PYBIND11_NOINLINE void add_base(const std::type_info *base, void *(*caster)(void *), const std::type_info* check_holder_type = nullptr) {
         auto base_info = detail::get_type_info(*base, false);
         if (!base_info) {
             std::string tname(base->name());
@@ -194,6 +194,13 @@ struct type_record {
                           "\" referenced unknown base type \"" + tname + "\"");
         }
 
+        if (check_holder_type && *check_holder_type != *(base_info->void_holder_type)) {
+            std::string tname(base->name());
+            detail::clean_type_id(tname);
+            pybind11_fail("generic_type: type \"" + std::string(name) +
+                          "\" and base base \"" + tname + "\" have different holder type");
+        }
+
         bases.append((PyObject *) base_info->type);
 
         if (base_info->type->tp_dictoffset != 0)

include/pybind11/cast.h

@@ -19,6 +19,31 @@
 NAMESPACE_BEGIN(pybind11)
 NAMESPACE_BEGIN(detail)
 
+/** Extract type_info of holder type (generalized to hold void)
+ *  Allows for comparisons of holder types between classes
+ *  Users may want to add specializations for custom smart pointers
+ *  in order to enable checks. */
+template <typename... Ts>
+struct void_holder {
+    static const std::type_info* type() {
+        return nullptr;
+    }
+};
+
+template <typename... Ts>
+struct void_holder<std::unique_ptr<Ts...>> {
+    static const std::type_info* type() {
+        return &typeid(std::unique_ptr<void>);
+    }
+};
+
+template <typename... Ts>
+struct void_holder<std::shared_ptr<Ts...>> {
+    static const std::type_info* type() {
+        return &typeid(std::shared_ptr<void>);
+    }
+};
+
 /// Additional type information which does not fit into the PyTypeObject
 struct type_info {
     PyTypeObject *type;
@@ -32,6 +57,9 @@ struct type_info {
     /** A simple type never occurs as a (direct or indirect) parent
      * of a class that makes use of multiple inheritance */
     bool simple_type = true;
+    /* holder_type generalized to hold void for base vs derived
+     * holder_type checks */
+    const std::type_info* void_holder_type = nullptr;
 };
 
 PYBIND11_NOINLINE inline internals &get_internals() {

include/pybind11/pybind11.h

@@ -1016,6 +1016,10 @@ class class_ : public detail::generic_type {
 
         detail::generic_type::initialize(&record);
 
+        /* Register holder type (e.g. std::shared_ptr) */
+        auto tinfo = detail::get_type_info(typeid(type));
+        tinfo->void_holder_type = detail::void_holder<holder_type>::type();
+
         if (has_alias) {
             auto &instances = pybind11::detail::get_internals().registered_types_cpp;
             instances[std::type_index(typeid(type_alias))] = instances[std::type_index(typeid(type))];
@@ -1026,7 +1030,7 @@ class class_ : public detail::generic_type {
     static void add_base(detail::type_record &rec) {
         rec.add_base(&typeid(Base), [](void *src) -> void * {
             return static_cast<Base *>(reinterpret_cast<type *>(src));
-        });
+        }, detail::void_holder<holder_type>::type());
     }
 
     template <typename Base, detail::enable_if_t<!is_base<Base>::value, int> = 0>

tests/CMakeLists.txt

@@ -68,6 +68,9 @@ endif()
 pybind11_add_module(pybind11_tests pybind11_tests.cpp
   ${PYBIND11_TEST_FILES} ${PYBIND11_HEADERS})
 
+pybind11_add_module(pybind11_test_broken_import pybind11_test_broken_import.cpp
+  ${PYBIND11_HEADERS})
+
 pybind11_enable_warnings(pybind11_tests)
 
 if(EIGEN3_FOUND)
@@ -80,9 +83,11 @@ set(testdir ${PROJECT_SOURCE_DIR}/tests)
 # Always write the output file directly into the 'tests' directory (even on MSVC)
 if(NOT CMAKE_LIBRARY_OUTPUT_DIRECTORY)
   set_target_properties(pybind11_tests PROPERTIES LIBRARY_OUTPUT_DIRECTORY ${testdir})
+  set_target_properties(pybind11_test_broken_import PROPERTIES LIBRARY_OUTPUT_DIRECTORY ${testdir})
   foreach(config ${CMAKE_CONFIGURATION_TYPES})
     string(TOUPPER ${config} config)
     set_target_properties(pybind11_tests PROPERTIES LIBRARY_OUTPUT_DIRECTORY_${config} ${testdir})
+    set_target_properties(pybind11_test_broken_import PROPERTIES LIBRARY_OUTPUT_DIRECTORY_${config} ${testdir})
   endforeach()
 endif()
 
@@ -97,8 +102,8 @@ if(NOT PYBIND11_PYTEST_FOUND)
 endif()
 
 # A single command to compile and run the tests
-add_custom_target(pytest COMMAND ${PYTHON_EXECUTABLE} -m pytest -rws ${PYBIND11_PYTEST_FILES}
-                  DEPENDS pybind11_tests WORKING_DIRECTORY ${testdir})
+add_custom_target(pytest COMMAND ${PYTHON_EXECUTABLE} -m pytest -rws ${PYBIND11_PYTEST_FILES} test_incompatible_holder.py
+                  DEPENDS pybind11_tests pybind11_test_broken_import WORKING_DIRECTORY ${testdir})
 
 if(PYBIND11_TEST_OVERRIDE)
   add_custom_command(TARGET pytest POST_BUILD

tests/pybind11_test_broken_import.cpp

@@ -0,0 +1,30 @@
+/*
+    tests/test_inheritance.cpp -- inheritance, automatic upcasting for polymorphic types
+
+    Copyright (c) 2016 Wenzel Jakob <[email protected]>
+
+    All rights reserved. Use of this source code is governed by a
+    BSD-style license that can be found in the LICENSE file.
+*/
+
+#include <pybind11/pybind11.h>
+
+class Base {
+};
+
+class Derived : public Base {
+};
+
+namespace py = pybind11;
+
+/** Note that this test cannot be part of the pybind11_tests module
+ *  because it is designed to fail on import */
+PYBIND11_PLUGIN(pybind11_test_broken_import) {
+    py::module m("pybind11_test_broken_import", "test for incompatible holder type");
+
+    py::class_<Base>(m, "Base");
+    py::class_<Derived, std::shared_ptr<Derived>, Base>(m, "Derived")
+        .def(py::init<>());
+
+    return m.ptr();
+}

tests/test_incompatible_holder.py

@@ -0,0 +1,7 @@
+import pytest
+
+
+def test_inheritance(msg):
+    with pytest.raises(ImportError):
+        from pybind11_test_broken_import import Base, Derived
+