diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h index 6d892a7de2..e2d3ca3218 100644 --- a/include/pybind11/detail/class.h +++ b/include/pybind11/detail/class.h @@ -601,8 +601,10 @@ extern "C" inline int pybind11_getbuffer(PyObject *obj, Py_buffer *view, int fla set_error(PyExc_BufferError, "Writable buffer requested for readonly storage"); return -1; } + + // Fill in all the information, and then downgrade as requested by the caller, or raise an + // error if that's not possible. view->obj = obj; - view->ndim = 1; // See discussion on PR #5407. view->internal = info; view->buf = info->ptr; view->itemsize = info->itemsize; @@ -610,17 +612,59 @@ extern "C" inline int pybind11_getbuffer(PyObject *obj, Py_buffer *view, int fla for (auto s : info->shape) { view->len *= s; } + view->ndim = (int) info->ndim; + view->shape = info->shape.data(); + view->strides = info->strides.data(); view->readonly = static_cast(info->readonly); if ((flags & PyBUF_FORMAT) == PyBUF_FORMAT) { view->format = const_cast(info->format.c_str()); } - if ((flags & PyBUF_ND) == PyBUF_ND) { - view->ndim = (int) info->ndim; - view->shape = info->shape.data(); - } - if ((flags & PyBUF_STRIDES) == PyBUF_STRIDES) { - view->strides = info->strides.data(); + + // Note, all contiguity flags imply PyBUF_STRIDES and lower. + if ((flags & PyBUF_C_CONTIGUOUS) == PyBUF_C_CONTIGUOUS) { + if (PyBuffer_IsContiguous(view, 'C') == 0) { + std::memset(view, 0, sizeof(Py_buffer)); + delete info; + set_error(PyExc_BufferError, + "C-contiguous buffer requested for discontiguous storage"); + return -1; + } + } else if ((flags & PyBUF_F_CONTIGUOUS) == PyBUF_F_CONTIGUOUS) { + if (PyBuffer_IsContiguous(view, 'F') == 0) { + std::memset(view, 0, sizeof(Py_buffer)); + delete info; + set_error(PyExc_BufferError, + "Fortran-contiguous buffer requested for discontiguous storage"); + return -1; + } + } else if ((flags & PyBUF_ANY_CONTIGUOUS) == PyBUF_ANY_CONTIGUOUS) { + if (PyBuffer_IsContiguous(view, 'A') == 0) { + std::memset(view, 0, sizeof(Py_buffer)); + delete info; + set_error(PyExc_BufferError, "Contiguous buffer requested for discontiguous storage"); + return -1; + } + + // If no strides are requested, the buffer must be C-contiguous. + // https://docs.python.org/3/c-api/buffer.html#contiguity-requests + } else if ((flags & PyBUF_STRIDES) != PyBUF_STRIDES) { + if (PyBuffer_IsContiguous(view, 'C') == 0) { + std::memset(view, 0, sizeof(Py_buffer)); + delete info; + set_error(PyExc_BufferError, + "C-contiguous buffer requested for discontiguous storage"); + return -1; + } + + view->strides = nullptr; + + // Since this is a contiguous buffer, it can also pretend to be 1D. + if ((flags & PyBUF_ND) != PyBUF_ND) { + view->shape = nullptr; + view->ndim = 1; + } } + Py_INCREF(view->obj); return 0; } diff --git a/tests/test_buffers.cpp b/tests/test_buffers.cpp index 11650546d3..3eb7df9464 100644 --- a/tests/test_buffers.cpp +++ b/tests/test_buffers.cpp @@ -167,6 +167,125 @@ TEST_SUBMODULE(buffers, m) { sizeof(float)}); }); + // A matrix that uses Fortran storage order. + class FortranMatrix : public Matrix { + public: + FortranMatrix(py::ssize_t rows, py::ssize_t cols) : Matrix(cols, rows) { + print_created(this, + std::to_string(rows) + "x" + std::to_string(cols) + " Fortran matrix"); + } + + float operator()(py::ssize_t i, py::ssize_t j) const { return Matrix::operator()(j, i); } + + float &operator()(py::ssize_t i, py::ssize_t j) { return Matrix::operator()(j, i); } + + using Matrix::data; + + py::ssize_t rows() const { return Matrix::cols(); } + py::ssize_t cols() const { return Matrix::rows(); } + }; + py::class_(m, "FortranMatrix", py::buffer_protocol()) + .def(py::init()) + + .def("rows", &FortranMatrix::rows) + .def("cols", &FortranMatrix::cols) + + /// Bare bones interface + .def("__getitem__", + [](const FortranMatrix &m, std::pair i) { + if (i.first >= m.rows() || i.second >= m.cols()) { + throw py::index_error(); + } + return m(i.first, i.second); + }) + .def("__setitem__", + [](FortranMatrix &m, std::pair i, float v) { + if (i.first >= m.rows() || i.second >= m.cols()) { + throw py::index_error(); + } + m(i.first, i.second) = v; + }) + /// Provide buffer access + .def_buffer([](FortranMatrix &m) -> py::buffer_info { + return py::buffer_info(m.data(), /* Pointer to buffer */ + {m.rows(), m.cols()}, /* Buffer dimensions */ + /* Strides (in bytes) for each index */ + {sizeof(float), sizeof(float) * size_t(m.rows())}); + }); + + // A matrix that uses a discontiguous underlying memory block. + class DiscontiguousMatrix : public Matrix { + public: + DiscontiguousMatrix(py::ssize_t rows, + py::ssize_t cols, + py::ssize_t row_factor, + py::ssize_t col_factor) + : Matrix(rows * row_factor, cols * col_factor), m_row_factor(row_factor), + m_col_factor(col_factor) { + print_created(this, + std::to_string(rows) + "(*" + std::to_string(row_factor) + ")x" + + std::to_string(cols) + "(*" + std::to_string(col_factor) + + ") matrix"); + } + + ~DiscontiguousMatrix() { + print_destroyed(this, + std::to_string(rows() / m_row_factor) + "(*" + + std::to_string(m_row_factor) + ")x" + + std::to_string(cols() / m_col_factor) + "(*" + + std::to_string(m_col_factor) + ") matrix"); + } + + float operator()(py::ssize_t i, py::ssize_t j) const { + return Matrix::operator()(i * m_row_factor, j * m_col_factor); + } + + float &operator()(py::ssize_t i, py::ssize_t j) { + return Matrix::operator()(i * m_row_factor, j * m_col_factor); + } + + using Matrix::data; + + py::ssize_t rows() const { return Matrix::rows() / m_row_factor; } + py::ssize_t cols() const { return Matrix::cols() / m_col_factor; } + py::ssize_t row_factor() const { return m_row_factor; } + py::ssize_t col_factor() const { return m_col_factor; } + + private: + py::ssize_t m_row_factor; + py::ssize_t m_col_factor; + }; + py::class_(m, "DiscontiguousMatrix", py::buffer_protocol()) + .def(py::init()) + + .def("rows", &DiscontiguousMatrix::rows) + .def("cols", &DiscontiguousMatrix::cols) + + /// Bare bones interface + .def("__getitem__", + [](const DiscontiguousMatrix &m, std::pair i) { + if (i.first >= m.rows() || i.second >= m.cols()) { + throw py::index_error(); + } + return m(i.first, i.second); + }) + .def("__setitem__", + [](DiscontiguousMatrix &m, std::pair i, float v) { + if (i.first >= m.rows() || i.second >= m.cols()) { + throw py::index_error(); + } + m(i.first, i.second) = v; + }) + /// Provide buffer access + .def_buffer([](DiscontiguousMatrix &m) -> py::buffer_info { + return py::buffer_info(m.data(), /* Pointer to buffer */ + {m.rows(), m.cols()}, /* Buffer dimensions */ + /* Strides (in bytes) for each index */ + {size_t(m.col_factor()) * sizeof(float) * size_t(m.cols()) + * size_t(m.row_factor()), + size_t(m.col_factor()) * sizeof(float)}); + }); + class BrokenMatrix : public Matrix { public: BrokenMatrix(py::ssize_t rows, py::ssize_t cols) : Matrix(rows, cols) {} @@ -274,6 +393,9 @@ TEST_SUBMODULE(buffers, m) { m.attr("PyBUF_ND") = PyBUF_ND; m.attr("PyBUF_STRIDES") = PyBUF_STRIDES; m.attr("PyBUF_INDIRECT") = PyBUF_INDIRECT; + m.attr("PyBUF_C_CONTIGUOUS") = PyBUF_C_CONTIGUOUS; + m.attr("PyBUF_F_CONTIGUOUS") = PyBUF_F_CONTIGUOUS; + m.attr("PyBUF_ANY_CONTIGUOUS") = PyBUF_ANY_CONTIGUOUS; m.def("get_py_buffer", [](const py::object &object, int flags) { Py_buffer buffer; diff --git a/tests/test_buffers.py b/tests/test_buffers.py index 3b2c893ec1..9fda879356 100644 --- a/tests/test_buffers.py +++ b/tests/test_buffers.py @@ -276,3 +276,80 @@ def test_to_pybuffer(): assert info.strides == [4 * info.itemsize, info.itemsize] assert info.suboffsets is None # Should be filled in here, but we don't use it. assert not info.readonly + + # A Fortran-shaped buffer can only be accessed at PyBUF_STRIDES level or higher. + mat = m.FortranMatrix(5, 4) + info = m.get_py_buffer(mat, m.PyBUF_STRIDES) + assert info.itemsize == ctypes.sizeof(ctypes.c_float) + assert info.len == mat.rows() * mat.cols() * info.itemsize + assert info.ndim == 2 + assert info.shape == [5, 4] + assert info.strides == [info.itemsize, 5 * info.itemsize] + assert info.suboffsets is None + assert not info.readonly + info = m.get_py_buffer(mat, m.PyBUF_INDIRECT) + assert info.itemsize == ctypes.sizeof(ctypes.c_float) + assert info.len == mat.rows() * mat.cols() * info.itemsize + assert info.ndim == 2 + assert info.shape == [5, 4] + assert info.strides == [info.itemsize, 5 * info.itemsize] + assert info.suboffsets is None # Should be filled in here, but we don't use it. + assert not info.readonly + + mat = m.DiscontiguousMatrix(5, 4, 2, 3) + info = m.get_py_buffer(mat, m.PyBUF_STRIDES) + assert info.itemsize == ctypes.sizeof(ctypes.c_float) + assert info.len == mat.rows() * mat.cols() * info.itemsize + assert info.ndim == 2 + assert info.shape == [5, 4] + assert info.strides == [2 * 4 * 3 * info.itemsize, 3 * info.itemsize] + assert info.suboffsets is None + assert not info.readonly + + +def test_to_pybuffer_contiguity(): + def check_strides(mat): + # The full block is memset to 0, so fill it with non-zero in real spots. + expected = np.arange(1, mat.rows() * mat.cols() + 1).reshape( + (mat.rows(), mat.cols()) + ) + for i in range(mat.rows()): + for j in range(mat.cols()): + mat[i, j] = expected[i, j] + # If all strides are correct, the exposed buffer should match the input. + np.testing.assert_array_equal(np.array(mat), expected) + + mat = m.Matrix(5, 4) + check_strides(mat) + # Should work in C-contiguous mode, but not Fortran order. + m.get_py_buffer(mat, m.PyBUF_C_CONTIGUOUS) + m.get_py_buffer(mat, m.PyBUF_ANY_CONTIGUOUS) + with pytest.raises(BufferError): + m.get_py_buffer(mat, m.PyBUF_F_CONTIGUOUS) + + mat = m.FortranMatrix(5, 4) + check_strides(mat) + # These flags imply C-contiguity, so won't work. + with pytest.raises(BufferError): + m.get_py_buffer(mat, m.PyBUF_SIMPLE) + with pytest.raises(BufferError): + m.get_py_buffer(mat, m.PyBUF_ND) + # Should work in Fortran-contiguous mode, but not C order. + with pytest.raises(BufferError): + m.get_py_buffer(mat, m.PyBUF_C_CONTIGUOUS) + m.get_py_buffer(mat, m.PyBUF_ANY_CONTIGUOUS) + m.get_py_buffer(mat, m.PyBUF_F_CONTIGUOUS) + + mat = m.DiscontiguousMatrix(5, 4, 2, 3) + check_strides(mat) + # Should never work. + with pytest.raises(BufferError): + m.get_py_buffer(mat, m.PyBUF_SIMPLE) + with pytest.raises(BufferError): + m.get_py_buffer(mat, m.PyBUF_ND) + with pytest.raises(BufferError): + m.get_py_buffer(mat, m.PyBUF_C_CONTIGUOUS) + with pytest.raises(BufferError): + m.get_py_buffer(mat, m.PyBUF_ANY_CONTIGUOUS) + with pytest.raises(BufferError): + m.get_py_buffer(mat, m.PyBUF_F_CONTIGUOUS)