`PySequence_GetItem` adjusting negative indexes make it difficult to handle them properly

Commencez par reproduire l’exemple Seq fourni via PySequence_GetItem, puis comparez le comportement avec des index négatifs décrit pour PySequence_SetItem et PySequence_DelItem. Examinez la direction proposée concernant Py_TPFLAGS_NO_SEQ_INDEX_ADJUST ainsi que la discussion associée ; le travail ne sera considéré comme terminé qu’une fois qu’une résolution aura été convenue quant à la manière dont ces APIs gèrent les index négatifs.

Bug report

Sequence types which attempt to handle negative indexes in __getitem__ experience double-corrected indexes when native code accesses them via PySequence_GetItem.

The same behaviour affects PySequence_SetItem and PySequence_DelItem, although not discussed further here.

See the ERROR comments in the snippet below:

from typing import Any, Iterable

# This is just a compiled wrapper to expose the PySequence_GetItem call 
# to Python. The point is to demonstrate that native code calling PySequence_GetItem
# interacts poorly with the sequence type Seq in this sample.
from native_module import pysequence_getitem

class Seq:
    """An immutable sequence type. This is implemented in pure python but it could be implemented
    in native code.
    """
    def __init__(self, values: Iterable[Any]):
        # store the sequence contents as {index -> value} mapping, because if we
        # used a list or tuple, that container would also handle negative indexes, confusing the example.
        self._items = dict(enumerate(values))

    def __len__(self) -> int:
        return len(self._items)

    def __getitem__(self, index: Any) -> Any:
        if isinstance(index, int):
            try:
                # Need to correct negative indexes
                corrected_index = index if index >= 0 else index + len(self._items)
                return self._items[corrected_index]
            except KeyError:
                # Raise the original index passed to __getitem__, ideally for better error message for users
                raise IndexError(index) from None
        else:
            # A real container would probably want to also implement slicing
            return NotImplemented()


seq = Seq("abcde")

print(seq[3])
print(seq[-3])
# raises IndexError(-10) as expected
# print(seq[-10])

print(pysequence_getitem(seq, 3))
print(pysequence_getitem(seq, -3))

# ERROR: should IndexError, but instead the -10 gets double-corrected
print(pysequence_getitem(seq, -10))

# ERROR: raises IndexError(-15), however the index requested was -20
print(pysequence_getitem(seq, -20))

(If requested I can push this as a sample repository)

Your environment

macOS 12.4, Python 3.12 main branch
I believe this negative index correction has been a feature of every Python 3 build on all OS variants.

Further background

PyO3 is a project to write native Python modules in Rust. To make the experience as straightforward as possible for Python developers, we aim to offer an API to create classes which matches Python as closely as possible.

PyO3 provides a way to implement __getitem__ which matches how user-defined Python classes work, like in the sample above. However, PySequence_GetItem's use of negative indexing has been a source of confusion: https://github.com/PyO3/pyo3/issues/2601#issuecomment-1231216542
https://github.com/PyO3/pyo3/issues/2392#issuecomment-1133955665

I am happy to be instructed that PyO3 should adopt a different design (ideas welcome). I would still defend it is reasonable to call the interaction demonstrated above for the Python class a bug.

I couldn't find any other discussion on this issue other than this comment from Guido in 2001, which implied that this behaviour was probably perceived as tricky even then: https://github.com/python/cpython/issues/34788#issuecomment-1093960713

Possible solutions

I believe it should be possible to add a new type flag, bikeshed it Py_TPFLAGS_NO_SEQ_INDEX_ADJUST, which disables this adjustment for PySequence_{Get,Set,Del}Item methods. Python-defined classes can have this flag set, avoiding the problematic behaviour displayed above. Native types can also use it to opt-out so that they can handle the negative index themselves. The main upside for native classes is that (if desired) the indexing operator would be able to raise IndexError containing the out-of-range index. E.g. (1, 2, 3)[-10] could raise IndexError(-10) instead of IndexError("tuple index out of range").