Obey contiguity requests for buffer protocol

If a contiguous buffer is requested, and the underlying buffer isn't,
then that should raise. This matches NumPy behaviour if you do something
like:
```
struct.unpack_from('5d', np.arange(20.0)[::4])  # Raises for contiguity
```

Also, if a buffer is contiguous, then it can masquerade as a
less-complex buffer, either by dropping strides, or even pretending to
be 1D. This matches NumPy behaviour if you do something like:
```
a = np.full((3, 5), 30.0)
struct.unpack_from('15d', a)  # --> Produces 1D tuple from 2D buffer.
```

Author: QuLogic

include/pybind11/detail/class.h

@@ -601,26 +601,66 @@ 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;
     view->len = view->itemsize;
     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<int>(info->readonly);
     if ((flags & PyBUF_FORMAT) == PyBUF_FORMAT) {
         view->format = const_cast<char *>(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')) {
+            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')) {
+            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')) {
+            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')) {
+            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;
 }

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_<FortranMatrix, Matrix>(m, "FortranMatrix", py::buffer_protocol())
+        .def(py::init<py::ssize_t, py::ssize_t>())
+
+        .def("rows", &FortranMatrix::rows)
+        .def("cols", &FortranMatrix::cols)
+
+        /// Bare bones interface
+        .def("__getitem__",
+             [](const FortranMatrix &m, std::pair<py::ssize_t, py::ssize_t> 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<py::ssize_t, py::ssize_t> 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_<DiscontiguousMatrix, Matrix>(m, "DiscontiguousMatrix", py::buffer_protocol())
+        .def(py::init<py::ssize_t, py::ssize_t, py::ssize_t, py::ssize_t>())
+
+        .def("rows", &DiscontiguousMatrix::rows)
+        .def("cols", &DiscontiguousMatrix::cols)
+
+        /// Bare bones interface
+        .def("__getitem__",
+             [](const DiscontiguousMatrix &m, std::pair<py::ssize_t, py::ssize_t> 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<py::ssize_t, py::ssize_t> 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;

tests/test_buffers.py

@@ -276,3 +276,78 @@ 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)