pybind / pybind11

Seamless operability between C++11 and Python
https://pybind11.readthedocs.io/
Other
15.09k stars 2.05k forks source link

[WIP] Bake smart_holder functionality into `class_` and `type_caster_base` #5213

Open rwgk opened 3 days ago

rwgk commented 3 days ago

Description

WORK IN PROGRESS

Best to ignore this PR until all tests pass.

High-level approach:

  1. Start with pybind11 master branch.
  2. Make smart_holder the default holder.
  3. Fix until all tests pass, while also adding in all tests from the smart_holder branch. (This is the hard part. Could take a while.)

Suggested changelog entry:

rwgk commented 3 days ago

Current state testing with the Google-internal toolchain, using ASAN:

7 failing tests:

//third_party/pybind11/tests:test_copy_move
//third_party/pybind11/tests:test_opaque_types
//third_party/pybind11/tests:test_smart_ptr
//third_party/pybind11/tests:test_stl
//third_party/pybind11/tests:test_stl_binders
//third_party/pybind11/tests:test_tagbased_polymorphic
//third_party/pybind11/tests:test_vector_unique_ptr_member

All other tests run ASAN-clean. For completeness, this is the list of passing tests:

40 passing tests:

//third_party/pybind11/tests:test_async
//third_party/pybind11/tests:test_buffers
//third_party/pybind11/tests:test_builtin_casters
//third_party/pybind11/tests:test_call_policies
//third_party/pybind11/tests:test_callbacks
//third_party/pybind11/tests:test_chrono
//third_party/pybind11/tests:test_class
//third_party/pybind11/tests:test_const_name
//third_party/pybind11/tests:test_constants_and_functions
//third_party/pybind11/tests:test_custom_type_casters
//third_party/pybind11/tests:test_custom_type_setup
//third_party/pybind11/tests:test_docstring_options
//third_party/pybind11/tests:test_eigen_matrix
//third_party/pybind11/tests:test_eigen_tensor
//third_party/pybind11/tests:test_enum
//third_party/pybind11/tests:test_eval
//third_party/pybind11/tests:test_exceptions
//third_party/pybind11/tests:test_factory_constructors
//third_party/pybind11/tests:test_gil_scoped
//third_party/pybind11/tests:test_iostream
//third_party/pybind11/tests:test_kwargs_and_defaults
//third_party/pybind11/tests:test_local_bindings
//third_party/pybind11/tests:test_methods_and_attributes
//third_party/pybind11/tests:test_modules
//third_party/pybind11/tests:test_multiple_inheritance
//third_party/pybind11/tests:test_numpy_array
//third_party/pybind11/tests:test_numpy_dtypes
//third_party/pybind11/tests:test_numpy_vectorize
//third_party/pybind11/tests:test_operator_overloading
//third_party/pybind11/tests:test_pickling
//third_party/pybind11/tests:test_python_multiple_inheritance
//third_party/pybind11/tests:test_pytypes
//third_party/pybind11/tests:test_sequences_and_iterators
//third_party/pybind11/tests:test_thread
//third_party/pybind11/tests:test_type_caster_pyobject_ptr
//third_party/pybind11/tests:test_union
//third_party/pybind11/tests:test_unnamed_namespace_a
//third_party/pybind11/tests:test_unnamed_namespace_b
//third_party/pybind11/tests:test_virtual_functions
//third_party/pybind11/tests:test_wip
rwgk commented 2 days ago

Tracking progress:

test_factory_constructors builds and runs successfully WITH ALL BAKEIN_BREAK removed.

This is still the same:

https://github.com/pybind/pybind11/pull/5213#issuecomment-2198362173

rwgk commented 14 hours ago

Tracking progress (@ commit 66a775eee9b1dff862f5b1af9dd9cdeb423c026c):

All existing tests pass, with just two obvious minor changes to the tests itself:

$ git diff master tests/test_class.cpp tests/test_smart_ptr.py
diff --git a/tests/test_class.cpp b/tests/test_class.cpp
index 9001d86b..e2981aca 100644
--- a/tests/test_class.cpp
+++ b/tests/test_class.cpp
@@ -606,11 +606,15 @@ CHECK_NOALIAS(8);
     static_assert(std::is_same<typename DoesntBreak##N::holder_type,                              \
                                std::TYPE##_ptr<BreaksBase<(N)>>>::value,                          \
                   "DoesntBreak" #N " has wrong holder_type!")
+#define CHECK_SMART_HOLDER(N)                                                                     \
+    static_assert(std::is_same<typename DoesntBreak##N::holder_type,                              \
+                               pybindit::memory::smart_holder>::value,                            \
+                  "DoesntBreak" #N " has wrong holder_type!")
 CHECK_HOLDER(1, unique);
 CHECK_HOLDER(2, unique);
 CHECK_HOLDER(3, unique);
-CHECK_HOLDER(4, unique);
-CHECK_HOLDER(5, unique);
+CHECK_SMART_HOLDER(4);
+CHECK_SMART_HOLDER(5);
 CHECK_HOLDER(6, shared);
 CHECK_HOLDER(7, shared);
 CHECK_HOLDER(8, shared);
diff --git a/tests/test_smart_ptr.py b/tests/test_smart_ptr.py
index bf0ae4ae..0d196199 100644
--- a/tests/test_smart_ptr.py
+++ b/tests/test_smart_ptr.py
@@ -298,6 +298,7 @@ def test_move_only_holder_with_addressof_operator():

 def test_smart_ptr_from_default():
+    pytest.skip("BAKEIN_EXPECTED: Failed: DID NOT RAISE <class 'RuntimeError'>")
     instance = m.HeldByDefaultHolder()
     with pytest.raises(RuntimeError) as excinfo:
         m.HeldByDefaultHolder.load_shared_ptr(instance)
rwgk commented 10 hours ago

Tracking progress (@ commit a332fe8cf4db42435e917a43e82b0775d6a42161):

GHA: all tests pass

ASAN, MSAN testing with Google-internal toolchain: all tests pass