bpo-45459: Rename buffer.h to pybuffer.h #31201
Conversation
Rename Include/buffer.h header file to Include/pybuffer.h to avoid conflits with projects having an existing buffer.h header file. * Incude pybuffer.h before object.h in Python.h. * Remove #include "buffer.h" from Include/cpython/object.h. * Add a forward declaration of the PyObject type in pybuffer.h to fix an inter-dependency issue.
| @@ -17,6 +17,9 @@ extern "C" { | |||
| * | |||
| */ | |||
|
|
|||
| // Forward declaration | |||
| typedef struct _object PyObject; | |||
There was a problem hiding this comment.
That's ugly. I had a forward declaration in my first PR. @encukou asked me to remove it.
There was a problem hiding this comment.
How is it "ugly"? Could you be more specific?
There are many similar code used in Python header files to fix inter-dependency issues. See for example Include/pystate.h.
There was a problem hiding this comment.
I didn't realize the reason it was there. It's ugly, but it might be a good way to solve the issue.
There was a problem hiding this comment.
I elaborated the comment to explain why this "Forward declaration" is needed. Is it better?
There was a problem hiding this comment.
Why not forward-define Py_buffer in object.h instead? That's a smaller change anyway, and removes the ordering requirement completely, rather than simply forcing it to happen in a certain way.
There was a problem hiding this comment.
I see Py_buffer as a C structure which is unrelated to Python objects. It's like a low-level thing "under" PyObject. In practice, it contains a PyObject* pointer and pybuffer.h contains other function definitions which use PyObject*.
I don't think that the order matters much, both orders work.
There was a problem hiding this comment.
Why not forward-define Py_buffer in object.h instead?
It's not possible: the Py_buffer struct has no name on purpose, see @tiran's comment.
|
When you're done making the requested changes, leave the comment: |
|
@tiran: Do you prefer to have a forward declaration of Py_buffer in object.h, or a forward declaration of PyObject in pybuffer.h? Well, I wrote my opinion on the order in a comment above, but I don't really care. Or if you prefer, I can put all of these definitions in a new single header file and make sure that it's one of the first included header. As I said, there are other header files using such forward declarations to break cycles of dependencies between two header files. In C, it's ok to use |
|
@encukou asked me to make Py_buffer an anon struct typedef. C does not support forward declaration of anon struct. You either have to give the Py_buffer struct a name again or keep your approach. |
Well, this PR works and doesn't require to give a name to the anonymous Py_buffer struct :-) @tiran: GitHub still tells me that there is "1 change requested" from your side. Can you either remove your -1 vote or vote +1 (approve)? |
|
@vstinner: Please replace |
|
I merged my fix. Thanks for reviews. It's unclear to me why the Py_buffer struct has no name, but well, I'm fine with no giving it a name :-) |
Rename Include/buffer.h header file to Include/pybuffer.h to avoid
conflits with projects having an existing buffer.h header file.
an inter-dependency issue.
https://bugs.python.org/issue45459