Skip to content

Instantly share code, notes, and snippets.

@antocuni
Last active May 19, 2020 09:59
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save antocuni/fd44ee897cc4cf14e874e2e686c60649 to your computer and use it in GitHub Desktop.
Save antocuni/fd44ee897cc4cf14e874e2e686c60649 to your computer and use it in GitHub Desktop.

To keep compatibility with CPython, we need to generate a trampoline for each method which uses one of HPy calling conventions, e.g.:

HPy_DEF_METH_VARARGS(add_ints)
static HPy add_ints_impl(HPyContext ctx, HPy self, HPy *args, HPy_ssize_t nargs);

After this, add_ints is a function of type HPyMeth (which is different in the CPython and the Universal cases), and can be put inside an HPyMethodDef[]. Morever, we can put old CPython-style methods by using an appropriate flag. Note the HPy_METH_* vs METH_* flags, and also note that if you specify the wrong variant, things will happily explode:

static PyObject *legacy_meth(PyObject *self, PyObject *args);

static HPyMethodDef FooMethods[] = {
    ...
    {"add_ints", add_ints, HPy_METH_VARARGS, ""},
    {"legacy_meth", legacy_meth, METH_VARARGS, ""},
    ...
}

Right now we have a different HPy_DEF_METH_* macro for the few calling conventions we support, but to fully support custom types, we need many more, for example newfunc, allocfunc, getattrfunc, etc. which are C typedefs stored in the various tp_new, tp_alloc, etc.

Before you ask: we need trampolines for all these functions to:

  1. "Insert" the HPyContext (CPython will call the trampoline without it, but the _impl function the user is going to write will need it)

  2. Convert PyObject * into HPy

  3. Convert the result

The easiest option is to add more macros and create HPy_DEF_METH_NEWFUNC & co., but I wonder whether we can do better.

To start with, I would like to have only ONE macro. This can be done with a preprocessor trick:

#define _HPy_DEF_METH_O(name) ...
#define _HPy_DEF_METH_VARARGS(name) ...
#define HPy_DEF_METH(kind, name) _HPy_DEF_##kind(name)

HPy_DEF_METH(METH_O, foo)
HPy_DEF_METH(METH_VARARGS, bar)

I think this is already an improvement over the existing situation. The quesion is, what is the best syntax to use? Some random proposals:

  • HPy_DEF_METH(METH_O, ...: I don't like the repetion of METH; moreover, METH_O is the CPython macro to indicate a legacy function, so you define your meth using METH_O but then you need to use HPy_METH_O in the method def table.

  • HPy_DEF_METH(HPy_METH_O, ...): this is a bit verbose, but it's more explicit.

  • HPy_DEF(HPy_METH_O, ...): is it better?

  • HPy_DEFINE(...)

  • HPy_DEF_METH(O, ...)

  • HPyMeth_DEF(...): I think I like this better than the current HPy_DEF_METH.

Keep in mind few facts:

  • We need to offer also the corresponding HPy_DECL_METH, which is needed if you want to put your methods in multiple *.c files. So any syntax we choose, must have a "nice" corresponding syntax for the declaration.

  • We need to use this syntax also for the various newfunc, allocfunc, etc. This is alredy a bit weird because METH_{NOARGS,O,VARARGS} are flags, while newfunc &co. are typedefs; so, for example:

    • HPyMeth_DEF(METH_O, ...) and HPyMeth_DEF(newfunc, ...) ?

The following proposal tries to solve/improve these issues, and goes a bit deeper than just renaming the macros. To start with, we should have a clear separation between the PyMethodDef[] (for legacy functions) and HPyMethodDef[], instead of relying on a flag which can be easily mistaken. For example:

static PyMethodDef LegacyMethods[] = {
    {"legacy1", legacy1, METH_NOARGS, ""},
    {"legacy2", legacy2, METH_O, ""},
    {NULL, NULL, 0, NULL}
};

static HPyMethodDef CoolMethods[] = {
    {"foo", foo, METH_NOARGS, ""},
    {"bar", bar, METH_O, ""},
    {NULL, NULL, 0, NULL}
};

static HPyModuleDef moduledef = {
    HPyModuleDef_HEAD_INIT,
    ...
    .m_legacy_methods = LegacyMethods,
    .m_methods = CoolMethods,
};

Among the other things, this has advantage of removing the distintion between METH_O and HPy_METH_O. The original idea of keeping .m_methods = ... was to make it as similar as possible to the existing syntax, to make porting easier without having to modify tons of place. However, my experience with ultrajson tells me that what you do in practice is to port methods one by one, so while you are at it moving the definition from one table to the other is not a big deal (plus, if we design our types correctly you would get a nice compilation error if you forget to move the method to the right table).

The main disadvantage of this is that HPyModule_Create can no longer be an alias for PyModule_Create in the CPython ABI mode, but it would need to be a custom function which creates the "real" PyModuleDef. But note that we need to implement it anyway for the cpython-universal case, and it's exactly the same sitation we already have for HPyType_FromSpec.

Once we decide to use our own HPyModuleDef, we can become creative, which brings more API design questions. For example, we could use unions to have a more type-safe way of specifying methods:

typedef HPy (*HPyMeth_NOARGS)(HPyContext, HPy);
typedef HPy (*HPyMeth_O)(HPyContext, HPy, HPy);

typedef struct {
    const char *name;
    union {
        void *pfunc;
        HPyMeth_NOARGS meth_noargs;
        HPyMeth_O meth_o;
    };
    int flags;
} HPyMethodDef;

...

static HPyMethodDef Methods[] = {
    {"foo", .meth_noargs = foo, METH_NOARGS},
    {"bar", .meth_o = bar, METH_O},
    {NULL, NULL, 0}
};

And the same for HPyType_Spec/HPyType_Slot:

typedef HPy (*HPy_newfunc)(HPyContext, HPy, HPy, HPy);
typedef HPy (*HPy_allocfunc)(HPyContext, HPy, HPy_ssize_t);

typedef struct {
    int slot;
    union {
        void *p;
        HPy_newfunc tp_new;
        HPy_allocfunc tp_alloc;
        // ...
    };
} HPyType_Slot;

static HPyType_Slot slots[] = {
       {Py_tp_new, .tp_new = Foo_New},
       {Py_tp_alloc, .tp_alloc = Foo_Alloc},
       {0, NULL},
};
  • The biggest advantage is that if you use a function with the wrong signature, you get a nice warning.

  • The biggest disadvantage is that setting the correct field of the union is obviously not enough: you still need to manually specify the correct flags, but at least it should be "visually evident" if something is wrong (e.g. if you specify {... .meth_o = foo, METH_NOARGS}). I don't think there is any way to get a nice compiler error/warning in this case: if you have any idea, please shout :)

If we go down this route, I suggest the following syntax for HPyMeth_DEFINE. The nice property is that there is a direct correspondence between the first argument of the macro and the typedef of the function pointer:

typedef HPy (*HPyMeth_NOARGS)(HPyContext, HPy);
typedef HPy (*HPyMeth_O)(HPyContext, HPy, HPy);
typedef HPy (*HPy_newfunc)(HPyContext, HPy, HPy, HPy);
typedef HPy (*HPy_allocfunc)(HPyContext, HPy, HPy_ssize_t);
// ...

HPyMeth_DEFINE(O, foo) // foo_impl is of type HPyMeth_O
HPyMeth_DEFINE(newfunc, Foo_New) // Foo_New_impl is of type HPyMeth_newfunc

Another nice property of this scheme is that we can extend it nicely if in the future we want to go towards an "argument-clinic-like" setup. E.g.:

// L_LDO means "returns a long and takes a long, a double and an object"
typedef long (*HPyMeth_L_LDO)(long, double, HPy);

HPyMeth_DEFINE(L_LDO, foo)
...

But I think that this email is already too long and too dense, so let's not discuss about "argument-clinic-like" now :)

Please let me know what you think!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment