Closed steve-s closed 1 year ago
rebased on master and fixed conflicts
I merged the documentation PR, and as predicted now this has conflicts, sorry.
just for reference: this PR resolves #397
Is there any way to banchmark this to see if it changes anything?
This should have an effect only on module creation/initialization, which I assumed is typically not on the hot-path, and it should not make it significantly more complex algorithmically, so it did not seem to me this needs extra performance check.
I merged the documentation PR, and as predicted now this has conflicts, sorry.
np, it's actually better. In the end, the conflicts pointed to exactly the things that I'd have to update anyway.
I've rebased the PR and updated the documentation. I tried to keep the already reviewed changes in one commit and added the new docs improvements in the new separate commit.
Since this changes the public API in a non-backward compatible way, it should increment the major API number, right?
Yes, that's a good question. My take was that until we finish the first milestone and release it, we allow ourselves to break binary compatibility.
IMO, we should just increase the major API version when releasing.
I don't think that we need or even should give compatibility guarantees on master
.
Thanks @steve-s
I think the best way to review is to start from the changes in the docs/examples and tests.
Make the multiphase module initialization the only way to initialize HPy modules, abandons the previous single phase initialization.
TL;DR: how module initialization looks like with this new API:
empty module: no user provided initialization function, no call to
HPyModule_Create
, just "register" theHPyModuleDef
inHPy_MODINIT
.when some module post-initialization is needed: use "exec" slot:
static HPyDef *moduledefs[] = { &exec1, NULL };
HPyModuleDef mod = { .defines = moduledefs, ... }
HPy_MODINIT(my_extension_name, mod)