GeezerVison

cc @zooba

Yep, this is the idea I had in mind. It looks weird until you remember that we intend for the spec to be static, which means each type will have its own callback that knows how to add additional members. You could use the type object to look up dynamic state, but that's not the intent of the entire API to begin with.

I honestly like this PR, and the PyType_Freeze function PR as well.

It's not clear what kind of restrictions are in place for the callback. For example, are you allowed to instantiate the type in the callback? (I imagine a singleton class could want to instantiate itself and set tp_new to NULL; what's keeping users from doing that?)
Whatever restrictions would be added can also be added to the code between PyType_From* and PyType_Freeze. The only advantage here is that PyType_From* knows the future value of Py_TPFLAGS_IMMUTABLETYPE; with PyType_Freeze we'll need another flag (or a different mechanism) if/when we need that.

Arbitrary code between PyType_From* and PyType_Freeze has the advantage of access to any local variables the user might have around as state. In general, a callback without a void* arg raises a red flag.

we intend for the spec to be static

That's most common, but they could also be filled dynamically and live on the stack. (And so you can't get a spec back from a type.)

It's not clear what kind of restrictions are in place for the callback. For example, are you allowed to instantiate the type in the callback?

Yeah, it's a good point, and it probably would be good to allow that if it's doable (at great risk of getting attribute errors in any code that may run). It's why my alternate proposals focused around working with the type dict, rather than the type object itself.

Arbitrary code between PyType_From* and PyType_Freeze has the advantage of access to any local variables the user might have around as state. In general, a callback without a void* arg raises a red flag.

Yeah, agreed, which is why I preferred providing an array of struct {const char *name; PyObject *value} in the slot that would be inserted into the type's dict.

The callback pointer could become a struct with a pointer and a userdata void*, but now we're likely getting to three layers of indirection for anyone to use it. The chances of messing that up are probably about equal with remembering to Py_DECREF everything in a list of PyObject, but I still think either makes it easier to avoid obscure semantic issues compared to being able to PyType_Freeze an arbitrary object at any arbitrary time.

Arbitrary code between PyType_From* and PyType_Freeze has the advantage of access to any local variables the user might have around as state. In general, a callback without a void* arg raises a red flag.

Honestly, I prefer PyType_Freeze() API. It's more natural and convenient, and I'm not convinced that Py_tp_create_callback brings much forward compatibility or is more "future proof". Well, that's my opinion :-)

I'm not convinced that Py_tp_create_callback brings much forward compatibility or is more "future proof".

It means we can change the way the type is constructed or allocated based on the fact that it's going to be immutable.

It also avoids adding a new API, which proves the forwards-compatibility of _FromSpec. Having to add a totally new API just means that one we thought was forwards-compatible actually wasn't, and so we need to make backwards-incompatible changes (modules that use PyType_Freeze can't even compile or load on older versions, whereas with only a type slot it can do a runtime version check to avoid actually passing it).

with only a type slot it can do a runtime version check to avoid actually passing it

That's a good point, thank you!

I don't think this point is worth replacing imperative code with a callback, though. We don't have a good track records with those: as years roll by, callbacks always seem to be needing a bit more context than they're getting.

my alternate proposals focused around working with the type dict, rather than the type object itself.

Modifying tp_dict using dict API is one of the more dangerous things you can do. Let's do less of that.
Alternately, the callback could return entries to add to the dict -- but that makes the callback a lot less powerful. OTOH,

providing an array of struct {const char *name; PyObject *value} in the slot that would be inserted into the type's dict.

I think this is a good idea regardless. Especially if we add a PyType_ApplySlots function.
If this array is enough for some type, it will be able to set Py_TPFLAGS_IMMUTABLETYPE in the initial FromSpec call, and get the future optimization you're thinking of. With runtime version checks!

The callback pointer could become a struct with a pointer and a userdata void*

... and now the spec isn't static any more :(

The C API Working Group approved PyType_Freeze() API: capi-workgroup/decisions#40 So I suggest to close this PR.

The C API Working Group approved PyType_Freeze() API: capi-workgroup/decisions#40 So I suggest to close this PR.

I think having both is good though as there can be use cases to having both options.

Especially if the Freeze call is not in itself part of the limited API but the spec slot is and having it internally call Freeze for the user in a forwards compatible way.

I close this PR since the C API Working Group approved PyType_Freeze() API: capi-workgroup/decisions#40