gh-130080: move _Py_EnsureArrayLargeEnough to a separate header so it can be used outside of the compiler #130930
Conversation
… so it can be used outside of the compiler
Include/internal/pycore_c_array.h
Outdated
| * | ||
| * Return 0 if successful and -1 (with exception set) otherwise. | ||
| */ | ||
| int _Py_c_array_EnsureCapacity(_Py_c_array_t *c_array, int idx); |
There was a problem hiding this comment.
| int _Py_c_array_EnsureCapacity(_Py_c_array_t *c_array, int idx); | |
| int | |
| _Py_c_array_EnsureCapacity(_Py_c_array_t *c_array, int idx); |
Should we assume idx is n int or maybe a uint32_t or just a size_t? any possibility of unsafe downcasting?
There was a problem hiding this comment.
Adding a struct for just the one function and no information hiding seems a bit of a waste.
What I had in mind, was a struct that could be used as a variable sized array/vector.
typedef struct _Py_c_array_t {
void *array;
int allocated; /* capacity of the array */
size_t item_size; /* size of each element */
}_Py_c_array_t;
int _Py_CArray_Init(_Py_c_array_t *array, Py_ssize_t itemsize, int initial_capacity);
int _Py_CArray_EnsureCapacity(_Py_c_array_t *array, int idx);
void _Py_CArray_Fini(_Py_c_array_t *array);
Although using it would require changing basicblock from:
cfg_instr *b_instr;
int b_ialloc;
to
_Py_c_array_t b_instr;
and changing every use of b_instr and b_ialloc, so you might not want to do that.
One other thing to consider is do you want a resizeable array or a vector (in C++ parlance)?
b_instr is conceptually a vector, we never leave gaps, and generally add at the end.
If you want to make it a vector, add a used field and a int _Py_CArray_Push(_Py_c_array_t *array, void *item); function.
| &b->b_ialloc, | ||
| DEFAULT_BLOCK_SIZE, | ||
| sizeof(cfg_instr))); | ||
| _Py_c_array_t array = { |
There was a problem hiding this comment.
Having to manually initialize a struct, just to pass it to a function seems clunky. The previous API was probably better.
There was a problem hiding this comment.
This is just to make the transition now. If we were defining basic block from scratch we would have put this struct field in it. But I think now that's a transformation for another PR.
It's a resizable array. The labelsmap is not a vector. |
… so it can be used outside of the compiler (python#130930)
… so it can be used outside of the compiler (python#130930)
This function is not specific to the compiler and I have a use case for it in another area of the codebase.