From 0640eb335977d31862dd6bc95df90a402039cdaf Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 18 May 2023 08:37:03 -0700 Subject: [PATCH] Use `static_assert()` `!std::is_pointer<>` to replace runtime guards. --- include/pybind11/detail/common.h | 5 +++++ include/pybind11/eigen/matrix.h | 27 ++++++++++++--------------- include/pybind11/eigen/tensor.h | 14 ++++---------- tests/test_eigen_matrix.cpp | 6 ------ tests/test_eigen_matrix.py | 17 ----------------- tests/test_eigen_tensor.inl | 4 ---- tests/test_eigen_tensor.py | 17 ----------------- 7 files changed, 21 insertions(+), 69 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index c2e6f9ec8..9e3ce60b4 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -1034,6 +1034,11 @@ struct format_descriptor< static std::string format() { return std::string(1, c); } }; +// Common message for `static_assert()`s, which are useful to easily preempt much less obvious +// errors in code that does not support `format_descriptor`. +#define PYBIND11_MESSAGE_POINTER_TYPES_ARE_NOT_SUPPORTED \ + "Pointer types (in particular `PyObject *`) are not supported." + PYBIND11_NAMESPACE_BEGIN(detail) // Returns the index of the given type in the type char array below, and in the list in numpy.h // The order here is: bool; 8 ints ((signed,unsigned)x(8,16,32,64)bits); float,double,long double; diff --git a/include/pybind11/eigen/matrix.h b/include/pybind11/eigen/matrix.h index 56241f43f..1f821476e 100644 --- a/include/pybind11/eigen/matrix.h +++ b/include/pybind11/eigen/matrix.h @@ -287,14 +287,11 @@ handle eigen_encapsulate(Type *src) { template struct type_caster::value>> { using Scalar = typename Type::Scalar; + static_assert(!std::is_pointer::value, + PYBIND11_MESSAGE_POINTER_TYPES_ARE_NOT_SUPPORTED); using props = EigenProps; bool load(handle src, bool convert) { - // dtype=object is not supported. See #1152 & #2259 for related experiments. - if (is_same_ignoring_cvref::value) { - return false; - } - // If we're in no-convert mode, only load if given an array of the correct type if (!convert && !isinstance>(src)) { return false; @@ -410,6 +407,9 @@ private: // Base class for casting reference/map/block/etc. objects back to python. template struct eigen_map_caster { + static_assert(!std::is_pointer::value, + PYBIND11_MESSAGE_POINTER_TYPES_ARE_NOT_SUPPORTED); + private: using props = EigenProps; @@ -462,6 +462,8 @@ private: using Type = Eigen::Ref; using props = EigenProps; using Scalar = typename props::Scalar; + static_assert(!std::is_pointer::value, + PYBIND11_MESSAGE_POINTER_TYPES_ARE_NOT_SUPPORTED); using MapType = Eigen::Map; using Array = array_t::value) { - return false; - } - // First check whether what we have is already an array of the right type. If not, we // can't avoid a copy (because the copy is also going to do type conversion). bool need_copy = !isinstance(src); @@ -614,6 +611,9 @@ private: // regular Eigen::Matrix, then casting that. template struct type_caster::value>> { + static_assert(!std::is_pointer::value, + PYBIND11_MESSAGE_POINTER_TYPES_ARE_NOT_SUPPORTED); + protected: using Matrix = Eigen::Matrix; @@ -642,16 +642,13 @@ public: template struct type_caster::value>> { using Scalar = typename Type::Scalar; + static_assert(!std::is_pointer::value, + PYBIND11_MESSAGE_POINTER_TYPES_ARE_NOT_SUPPORTED); using StorageIndex = remove_reference_t().outerIndexPtr())>; using Index = typename Type::Index; static constexpr bool rowMajor = Type::IsRowMajor; bool load(handle src, bool) { - // dtype=object is not supported. See #1152 & #2259 for related experiments. - if (is_same_ignoring_cvref::value) { - return false; - } - if (!src) { return false; } diff --git a/include/pybind11/eigen/tensor.h b/include/pybind11/eigen/tensor.h index 4bdda0da4..d067003a7 100644 --- a/include/pybind11/eigen/tensor.h +++ b/include/pybind11/eigen/tensor.h @@ -164,16 +164,13 @@ PYBIND11_WARNING_POP template struct type_caster::ValidType> { + static_assert(!std::is_pointer::value, + PYBIND11_MESSAGE_POINTER_TYPES_ARE_NOT_SUPPORTED); using Helper = eigen_tensor_helper; static constexpr auto temp_name = get_tensor_descriptor::value; PYBIND11_TYPE_CASTER(Type, temp_name); bool load(handle src, bool convert) { - // dtype=object is not supported. See #1152 & #2259 for related experiments. - if (is_same_ignoring_cvref::value) { - return false; - } - if (!convert) { if (!isinstance(src)) { return false; @@ -364,15 +361,12 @@ struct get_storage_pointer_type struct type_caster, typename eigen_tensor_helper>::ValidType> { + static_assert(!std::is_pointer::value, + PYBIND11_MESSAGE_POINTER_TYPES_ARE_NOT_SUPPORTED); using MapType = Eigen::TensorMap; using Helper = eigen_tensor_helper>; bool load(handle src, bool /*convert*/) { - // dtype=object is not supported. See #1152 & #2259 for related experiments. - if (is_same_ignoring_cvref::value) { - return false; - } - // Note that we have a lot more checks here as we want to make sure to avoid copies if (!isinstance(src)) { return false; diff --git a/tests/test_eigen_matrix.cpp b/tests/test_eigen_matrix.cpp index ab5a658f4..554cc4d7f 100644 --- a/tests/test_eigen_matrix.cpp +++ b/tests/test_eigen_matrix.cpp @@ -425,10 +425,4 @@ TEST_SUBMODULE(eigen_matrix, m) { py::module_::import("numpy").attr("ones")(10); return v[0](5); }); - - m.def("pass_eigen_matrix_dtype_object", - [](const Eigen::Matrix &) {}); - m.def("pass_eigen_ref_matrix_dtype_object", - [](const Eigen::Ref> &) {}); - m.def("pass_eigen_sparse_matrix_dtype_object", [](const Eigen::SparseMatrix &) {}); } diff --git a/tests/test_eigen_matrix.py b/tests/test_eigen_matrix.py index eccb9f8a2..b2e76740b 100644 --- a/tests/test_eigen_matrix.py +++ b/tests/test_eigen_matrix.py @@ -805,20 +805,3 @@ def test_custom_operator_new(): o = m.CustomOperatorNew() np.testing.assert_allclose(o.a, 0.0) np.testing.assert_allclose(o.b.diagonal(), 1.0) - - -@pytest.mark.parametrize( - "pass_eigen_type_dtype_object", - [ - m.pass_eigen_matrix_dtype_object, - m.pass_eigen_ref_matrix_dtype_object, - m.pass_eigen_sparse_matrix_dtype_object, - ], -) -def test_pass_array_with_dtype_object(pass_eigen_type_dtype_object): - # Only the dtype matters (not shape etc.): dtype=object is (should be) the - # first check in the type_caster load() implementations. - obj = np.array([], dtype=object) - with pytest.raises(TypeError) as excinfo: - pass_eigen_type_dtype_object(obj) - assert "incompatible function arguments" in str(excinfo.value) diff --git a/tests/test_eigen_tensor.inl b/tests/test_eigen_tensor.inl index 27d7e86b2..d864ce737 100644 --- a/tests/test_eigen_tensor.inl +++ b/tests/test_eigen_tensor.inl @@ -320,10 +320,6 @@ void init_tensor_module(pybind11::module &m) { "round_trip_rank_0_view", [](Eigen::TensorMap> &tensor) { return tensor; }, py::return_value_policy::reference); - - m.def("pass_eigen_tensor_dtype_object", [](const Eigen::Tensor &) {}); - m.def("pass_eigen_tensor_map_dtype_object", - [](Eigen::TensorMap> &) {}); } void test_module(py::module_ &m) { diff --git a/tests/test_eigen_tensor.py b/tests/test_eigen_tensor.py index 05842acee..3e7ee6b7f 100644 --- a/tests/test_eigen_tensor.py +++ b/tests/test_eigen_tensor.py @@ -286,20 +286,3 @@ def test_doc_string(m, doc): f"round_trip_const_view_tensor(arg0: numpy.ndarray[numpy.float64[?, ?, ?], {order_flag}])" " -> numpy.ndarray[numpy.float64[?, ?, ?]]" ) - - -@pytest.mark.parametrize("m", submodules) -@pytest.mark.parametrize( - "func_name", - [ - "pass_eigen_tensor_dtype_object", - "pass_eigen_tensor_map_dtype_object", - ], -) -def test_pass_array_with_dtype_object(m, func_name): - # Only the dtype matters (not shape etc.): dtype=object is (should be) the - # first check in the type_caster load() implementations. - obj = np.array([], dtype=object) - with pytest.raises(TypeError) as excinfo: - getattr(m, func_name)(obj) - assert "incompatible function arguments" in str(excinfo.value)