Closed fangerer closed 1 year ago
Why do we need the calls to
HPyStructSequenceBuilder_Set*
? Since the spec is passed in, couldn't the call toHPyStructSequenceBuilder_New
fill in the fields?
We could, ofc, do that but it's a question of convenience. I think in Numpy it wouldn't be a big deal. This is how we use it in Numpy/HPy right now (in file typeinfo.c):
HPyStructSequenceBuilder entry = HPyStructSequenceBuilder_New(ctx, PyArray_typeinfoType);
HPyStructSequenceBuilder_Set_i(ctx, entry, 0, typechar);
HPyStructSequenceBuilder_Set_i(ctx, entry, 1, typenum);
HPyStructSequenceBuilder_Set_i(ctx, entry, 2, nbits);
HPyStructSequenceBuilder_Set_i(ctx, entry, 3, align);
HPyStructSequenceBuilder_Set(ctx, entry, 4, type_obj);
In this case, you would need to allocate a temporary array (could be on stack).
You are probably right, we should decouple the builder and creating the actual instance. I'll introduce a HPyStructSequence_New(HPyContext *ctx, HPy type, HPy_ssize_t n, HPy const *args)
to be able to create the instance from an array.
I'll probably just kill the builder for now since I don't see a big use case for that right now.
@mattip I've removed HPyStructSequence_NewFromFormat
(which also removes usage of _malloca
). As already mentioned, the structseq API cannot be simpler than that.
This PR also includes the promised fixup for #416 (see https://github.com/hpyproject/hpy/pull/415/commits/4c5f6f19ebb28235a86c247553b1b20693643b27).
Please do (a hopefully) last review round 🙂.
Thanks @fangerer
This PR adds some HPy helper functions to work with struct sequences. Since CPython's struct sequences and namedtuple are very similar, the idea is to map the API to CPython's
PyStructSequence_*
functions in CPython ABI mode and to usenamedtuple
in universal ABI mode.However, there are subtle differences regarding the hidden elements and the error behavior. HPy's struct sequence API is therefore a little bit smaller.
I admit, the implementation for the universal mode doesn't look very nice but the big advantage is that this won't need any more implementation on the runtime side.