[BUG] Call to bytes.__contains__(self) is wrongly unconditionally translated to PySequence_Contains(self)

[navytux]

Describe the bug
Hello up there. I've hit a situation where call to bytes.__contains__(self) is unconditionally translated to PySequence_Contains(self) even if type of self inherits from bytes and provides its own .__contains__. This incorrectly leads to RecursionError in my case. Please find details below.

To Reproduce
Code to reproduce the behaviour:

# cython: language_level=3

class MyBytes(bytes):
    def __contains__(self, key):
        print('__contains__')
        return bytes.__contains__(self, key)

x = MyBytes(b'zzz')
b'a' in x

Running it through pure python (both py2 and py3) works as expected:

$ python containsbug.pyx
__contains__

However running it through Cython leads to RecursionError:

$ cythonize -i containsbug.pyx && python -c 'import containsbug'
Compiling /home/kirr/src/tools/go/pygolang/containsbug.pyx because it changed.
[1/1] Cythonizing /home/kirr/src/tools/go/pygolang/containsbug.pyx
running build_ext
building 'containsbug' extension
creating /home/kirr/src/tools/go/pygolang/tmpgzlwnd7x/home
creating /home/kirr/src/tools/go/pygolang/tmpgzlwnd7x/home/kirr
creating /home/kirr/src/tools/go/pygolang/tmpgzlwnd7x/home/kirr/src
creating /home/kirr/src/tools/go/pygolang/tmpgzlwnd7x/home/kirr/src/tools
creating /home/kirr/src/tools/go/pygolang/tmpgzlwnd7x/home/kirr/src/tools/go
creating /home/kirr/src/tools/go/pygolang/tmpgzlwnd7x/home/kirr/src/tools/go/pygolang
x86_64-linux-gnu-gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O2 -Wall -g -ffile-prefix-map=/build/python3.9-RNBry6/python3.9-3.9.2=. -fstack-protector-strong -Wformat -Werror=format-security -g -fwrapv -O2 -g -ffile-prefix-map=/build/python3.9-RNBry6/python3.9-3.9.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC -I/home/kirr/src/wendelin/venv/py3.venv/include -I/usr/include/python3.9 -c /home/kirr/src/tools/go/pygolang/containsbug.c -o /home/kirr/src/tools/go/pygolang/tmpgzlwnd7x/home/kirr/src/tools/go/pygolang/containsbug.o
x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-z,relro -g -fwrapv -O2 -Wl,-z,relro -g -fwrapv -O2 -g -ffile-prefix-map=/build/python3.9-RNBry6/python3.9-3.9.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 /home/kirr/src/tools/go/pygolang/tmpgzlwnd7x/home/kirr/src/tools/go/pygolang/containsbug.o -o /home/kirr/src/tools/go/pygolang/containsbug.cpython-39-x86_64-linux-gnu.so
__contains__
__contains__
__contains__
__contains__
...
__contains__
__contains__
__contains__
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "containsbug.pyx", line 9, in init containsbug
    b'a' in x
  File "containsbug.pyx", line 6, in containsbug.MyBytes.__contains__
    return bytes.__contains__(self, key)
  File "containsbug.pyx", line 6, in containsbug.MyBytes.__contains__
    return bytes.__contains__(self, key)
  File "containsbug.pyx", line 6, in containsbug.MyBytes.__contains__
    return bytes.__contains__(self, key)
  [Previous line repeated 985 more times]
  File "containsbug.pyx", line 5, in containsbug.MyBytes.__contains__
    print('__contains__')
RecursionError: maximum recursion depth exceeded while calling a Python object
__contains__

Expected behavior
The same output as when run through pure python.

Environment (please complete the following information):

• OS: Debian 11 GNU/Linux
• Python version: 3.9.2, 2.7.18
• Cython version: 0.29.28-26-gac29e8976 (today's 0.29.x)

Additional context

The code for bytes.__contains__(self) is translated as follows:

  /* "containsbug.pyx":6
 *     def __contains__(self, key):
 *         print('__contains__')
 *         return bytes.__contains__(self, key)             # <<<<<<<<<<<<<<
 * 
 * x = MyBytes(b'zzz')
 */
  __Pyx_XDECREF(__pyx_r);
  if (unlikely(__pyx_v_self == Py_None)) {
    PyErr_Format(PyExc_TypeError, "descriptor '%s' requires a '%s' object but received a 'NoneType'", "__contains__", "bytes");
    __PYX_ERR(0, 6, __pyx_L1_error)
  }
  if (!(likely(PyBytes_Check(__pyx_v_self))||(PyErr_Format(PyExc_TypeError, "Expected %.16s, got %.200s", "bytes", Py_TYPE(__pyx_v_self)->tp_name), 0))) __PYX_ERR(0, 6, __pyx_L1_error)
  __pyx_t_2 = PySequence_Contains(((PyObject*)__pyx_v_self), __pyx_v_key); if (unlikely(__pyx_t_2 == ((int)-1))) __PYX_ERR(0, 6, __pyx_L1_error)
  __pyx_t_1 = __Pyx_PyBool_FromLong(__pyx_t_2); if (unlikely(!__pyx_t_1)) __PYX_ERR(0, 6, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_1);
  __pyx_r = __pyx_t_1;
  __pyx_t_1 = 0;
  goto __pyx_L0;

NOTE PySequence_Contains(self) call there which calls MyBytes.__contains__ again and leads to recursion.

Thanks beforehand,
Kirill

[da-woods]

I wonder if all of the __contains__: PySequence_Contains lines in Builtin.py should be deleted?

cython/Cython/Compiler/Builtin.py

Line 276 in 89591f7

("bytes", "PyBytes_Type", [BuiltinMethod("__contains__", "TO", "b", "PySequence_Contains"),

[navytux]

@da-woods, thanks. I do not know Cython internals, but probably you are right - as it is now __contains__ optimization is incorrect.

[navytux]

@da-woods, @scoder, thanks once again.