gh-121115: Skip __index__ in PyLong_AsNativeBytes by default #121118
Conversation
Co-authored-by: Nice Zombies <nineteendo19d0@gmail.com>
| passed, its :meth:`~object.__index__` method will be called first. This may | ||
| result in Python code executing and other threads being allowed to run, which | ||
| could cause changes to other objects or values in use. When *flags* is | ||
| ``-1``, this option is not set, and non-integer values will raise |
There was a problem hiding this comment.
I would expect it to be set by default.
There was a problem hiding this comment.
The default behaviour is defined as "most like C", and this is unlike C. Having it enabled by default also risks causing crashes, whereas having it not set only results in a TypeError.
But I initially started expecting it to be set by default too. It only took a bit of thinking about it to convince myself otherwise.
There was a problem hiding this comment.
What does "most like C" mean in context of conversion from Python integer to C integer? I expected it to be like most common converters PyLong_AsLong() and PyArg_Parse("i"), but seems I understand it wrong.
__index__ should be called by default, because the purpose of the __index__ method is that integer-like object should be able to substitute a real int in almost all cases.
There was a problem hiding this comment.
What does "most like C" mean in context of conversion from Python integer to C integer?
It's deliberately vague, but a C cast is not going to trigger other code to run (unlike a C++ cast), and it's going to succeed even if it truncated the value and lost information. There are ways to detect this, and the existing functions force you to detect it, but AsNativeBytes deliberately allows you to bypass those if you know what you're doing.
__index__should be called by default, because the purpose of the__index__method is that integer-like object should be able to substitute a real int in almost all cases.
As a low level API, this can go in the "almost" situation. If this goes in, then Victor's PR with higher-level APIs for specific types should start passing this flag to get that behaviour.
As I said above, if you call this wrong and the default is to reenter Python, you'll get a hard crash. If you call this wrong and the default is to raise TypeError, you get a safe exception. We should choose the safe path for the default. (I think we should choose it for the other APIs as well, but am willing to concede different logic for high/low level APIs.)
There was a problem hiding this comment.
My concern is that users which will use this API for concrete C integer type will miss such detail and produce extensions that only work with int and subclasses, but not with other integer-like objects.
I have no objection if this is the intention.
There was a problem hiding this comment.
Yeah, that's the intent. Their users will get a TypeError when it matters and can request a change.
|
Thanks @zooba for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13. |
…ythonGH-121118) (cherry picked from commit 2894aa1) Co-authored-by: Steve Dower <steve.dower@python.org>
|
GH-121133 is a backport of this pull request to the 3.13 branch. |
📚 Documentation preview 📚: https://cpython-previews--121118.org.readthedocs.build/