Missing C fields?

[hannosch]

I've been looking over tp_flags and what these imply in different Python versions.

It looks like we declare Py_TPFLAGS_DEFAULT, which implies Py_TPFLAGS_HAVE_INPLACEOPS.

That in turn says PySequenceMethods should have the sq_inplace_concat and sq_inplace_repeat fields. Our method definition ends with sq_contains. Our PyNumberMethods defines all the inplace fields. The inplace operators were added in Python 2.0.

Py_TPFLAGS_DEFAULT also implies Py_TPFLAGS_HAVE_CLASS, which wants a whole host of fields. We now define all of them up to tp_new, but miss the rest of them:

tp_free,
tp_is_gc,
tp_bases,
tp_mro,
tp_cache,
tp_subclasses,
tp_weaklist

Looking over the tp_flags for Python 3.6, there's only one new flag worth mentioning Py_TPFLAGS_HAVE_FINALIZE. This is a new addition as of Python 3.4, where they reworked how destructors worked.

In Python 3.5 the tp_compare flag got repurposed as tp_as_async, but we luckily have set that one to 0.

I'm not quite sure what to do about these. I think we can ignore the new opt-in finalize and async flags.

Do we need to do something about the new-style classes additions (Py_TPFLAGS_HAVE_CLASS)?

The inplace sequence methods seem like something we maybe should wrap and add to both the Python and C code.

@stephan-hof Thoughts?

[hannosch]

Ok, forget about the inplace sequence issue. On the Python level there's no extra double under method for them, but instead the number special methods (e.g. __iadd__) get used.

On the C level there's some magic, which also seems to re-use the number specific functions. Some yet unresolved complicated interaction is mentioned in http://bugs.python.org/issue11477.

[stephan-hof]

Hi,

regarding these fields

tp_free,
tp_is_gc,
tp_bases,
tp_mro,
tp_cache,
tp_subclasses,
tp_weaklist

I don't see that we have to worry about them. If they are not set, python automatically sets good defaults when PyType_Ready is called.

For the inplace sequence operations. First of all, python double checks if they are set to NULL, so python does not blindly rely on the flag only. Looking through the source all access to this fields were protected via sq_inplace_concat != NULL.
The only drawback of the current implementation (leaving it NULL) is, if Acquistion wraps an object which has (only) this slot set. Which can only be an object defined in a C-Extension. In that case proxying does not happen.
However I don't see how to fix this easily. This has to be set on the type, so as soon as its defined all the users of it see 'it is not NULL' => looks like it is implemented so lets call it. Unfortunately on type definition we don't know what kind of object we wrap.
If we implement that on our types we must return Py_NotImplemented if the wrapped object does not have the slot set, however right now I cannot judge if this a valid way to go or not.
What do you think how much we have to care about this case anyway ?
It only appears if there is a type defined in a C-Extensions + implementing only this slot.
For the 'normal' cases where __iadd__ is defined on the python level the current mechanics (falling back to the number slots) work.

Py_TPFLAGS_HAVE_FINALIZE, do you see a case for implementing/using it ?
The current tp_dealloc in Acquisition does not need 'Safe object finalization'. Leaving it NULL is fine if the flag is not set.

tp_as_async here I could image that we wrap these calls so that the python methods

__aiter__
__anext__
__await__

are called with a acquisition wrapped self.

[hannosch]

Hi.

Sorry, I didn't fully appreciate what the PyTypeReady and friends were doing.

I think we can ignore sq_inplace_concat and leave it as is, same for Py_TPFLAGS_HAVE_FINALIZE. You are a better judge of whether or not changes are desirable here. I just brought them up as things that have changed since Acquisition.c was first written, and where it wasn't immediately obvious someone had thought about them.

For tp_as_async, I'd agree that we could implement it. Though that's a fairly low priority feature.