bpo-31572: Get rid of PyObject_HasAttr() and _PyObject_HasAttrId() in the _io module. (#3726)

Author: serhiy-storchaka

Modules/_io/_iomodule.c

@@ -36,6 +36,7 @@ PyObject *_PyIO_str_getstate = NULL;
 PyObject *_PyIO_str_isatty = NULL;
 PyObject *_PyIO_str_newlines = NULL;
 PyObject *_PyIO_str_nl = NULL;
+PyObject *_PyIO_str_peek = NULL;
 PyObject *_PyIO_str_read = NULL;
 PyObject *_PyIO_str_read1 = NULL;
 PyObject *_PyIO_str_readable = NULL;
@@ -740,6 +741,7 @@ PyInit__io(void)
     ADD_INTERNED(getstate)
     ADD_INTERNED(isatty)
     ADD_INTERNED(newlines)
+    ADD_INTERNED(peek)
     ADD_INTERNED(read)
     ADD_INTERNED(read1)
     ADD_INTERNED(readable)

Modules/_io/_iomodule.h

@@ -164,6 +164,7 @@ extern PyObject *_PyIO_str_getstate;
 extern PyObject *_PyIO_str_isatty;
 extern PyObject *_PyIO_str_newlines;
 extern PyObject *_PyIO_str_nl;
+extern PyObject *_PyIO_str_peek;
 extern PyObject *_PyIO_str_read;
 extern PyObject *_PyIO_str_read1;
 extern PyObject *_PyIO_str_readable;

Modules/_io/bufferedio.c

@@ -1521,7 +1521,7 @@ static PyObject *
 _bufferedreader_read_all(buffered *self)
 {
     Py_ssize_t current_size;
-    PyObject *res = NULL, *data = NULL, *tmp = NULL, *chunks = NULL;
+    PyObject *res = NULL, *data = NULL, *tmp = NULL, *chunks = NULL, *readall;
 
     /* First copy what we have in the current buffer. */
     current_size = Py_SAFE_DOWNCAST(READAHEAD(self), Py_off_t, Py_ssize_t);
@@ -1541,32 +1541,28 @@ _bufferedreader_read_all(buffered *self)
     }
     _bufferedreader_reset_buf(self);
 
-    if (PyObject_HasAttr(self->raw, _PyIO_str_readall)) {
-        tmp = PyObject_CallMethodObjArgs(self->raw, _PyIO_str_readall, NULL);
+    readall = _PyObject_GetAttrWithoutError(self->raw, _PyIO_str_readall);
+    if (readall) {
+        tmp = _PyObject_CallNoArg(readall);
+        Py_DECREF(readall);
         if (tmp == NULL)
             goto cleanup;
         if (tmp != Py_None && !PyBytes_Check(tmp)) {
             PyErr_SetString(PyExc_TypeError, "readall() should return bytes");
             goto cleanup;
         }
-        if (tmp == Py_None) {
-            if (current_size == 0) {
-                res = Py_None;
-                goto cleanup;
-            } else {
-                res = data;
-                goto cleanup;
+        if (current_size == 0) {
+            res = tmp;
+        } else {
+            if (tmp != Py_None) {
+                PyBytes_Concat(&data, tmp);
             }
-        }
-        else if (current_size) {
-            PyBytes_Concat(&data, tmp);
             res = data;
-            goto cleanup;
-        }
-        else {
-            res = tmp;
-            goto cleanup;
         }
+        goto cleanup;
+    }
+    else if (PyErr_Occurred()) {
+        goto cleanup;
     }
 
     chunks = PyList_New(0);

Modules/_io/iobase.c

@@ -68,11 +68,9 @@ PyDoc_STRVAR(iobase_doc,
    by whatever subclass. */
 
 _Py_IDENTIFIER(__IOBase_closed);
-#define IS_CLOSED(self) \
-    _PyObject_HasAttrId(self, &PyId___IOBase_closed)
-
 _Py_IDENTIFIER(read);
 
+
 /* Internal methods */
 static PyObject *
 iobase_unsupported(const char *message)
@@ -131,6 +129,24 @@ iobase_truncate(PyObject *self, PyObject *args)
     return iobase_unsupported("truncate");
 }
 
+static int
+iobase_is_closed(PyObject *self)
+{
+    PyObject *res;
+    /* This gets the derived attribute, which is *not* __IOBase_closed
+       in most cases! */
+    res = _PyObject_GetAttrId(self, &PyId___IOBase_closed);
+    if (res == NULL) {
+        if (!PyErr_ExceptionMatches(PyExc_AttributeError)) {
+            return -1;
+        }
+        PyErr_Clear();
+        return 0;
+    }
+    Py_DECREF(res);
+    return 1;
+}
+
 /* Flush and close methods */
 
 /*[clinic input]
@@ -146,45 +162,60 @@ _io__IOBase_flush_impl(PyObject *self)
 /*[clinic end generated code: output=7cef4b4d54656a3b input=773be121abe270aa]*/
 {
     /* XXX Should this return the number of bytes written??? */
-    if (IS_CLOSED(self)) {
+    int closed = iobase_is_closed(self);
+
+    if (!closed) {
+        Py_RETURN_NONE;
+    }
+    if (closed > 0) {
         PyErr_SetString(PyExc_ValueError, "I/O operation on closed file.");
+    }
+    return NULL;
+}
+
+static PyObject *
+iobase_closed_get(PyObject *self, void *context)
+{
+    int closed = iobase_is_closed(self);
+    if (closed < 0) {
         return NULL;
     }
-    Py_RETURN_NONE;
+    return PyBool_FromLong(closed);
 }
 
 static int
-iobase_closed(PyObject *self)
+iobase_check_closed(PyObject *self)
 {
     PyObject *res;
     int closed;
     /* This gets the derived attribute, which is *not* __IOBase_closed
        in most cases! */
-    res = PyObject_GetAttr(self, _PyIO_str_closed);
-    if (res == NULL)
+    res = _PyObject_GetAttrWithoutError(self, _PyIO_str_closed);
+    if (res == NULL) {
+        if (PyErr_Occurred()) {
+            return -1;
+        }
         return 0;
+    }
     closed = PyObject_IsTrue(res);
     Py_DECREF(res);
-    return closed;
-}
-
-static PyObject *
-iobase_closed_get(PyObject *self, void *context)
-{
-    return PyBool_FromLong(IS_CLOSED(self));
+    if (closed <= 0) {
+        return closed;
+    }
+    PyErr_SetString(PyExc_ValueError, "I/O operation on closed file.");
+    return -1;
 }
 
 PyObject *
 _PyIOBase_check_closed(PyObject *self, PyObject *args)
 {
-    if (iobase_closed(self)) {
-        PyErr_SetString(PyExc_ValueError, "I/O operation on closed file.");
+    if (iobase_check_closed(self)) {
         return NULL;
     }
-    if (args == Py_True)
+    if (args == Py_True) {
         return Py_None;
-    else
-        Py_RETURN_NONE;
+    }
+    Py_RETURN_NONE;
 }
 
 /* XXX: IOBase thinks it has to maintain its own internal state in
@@ -204,9 +235,14 @@ _io__IOBase_close_impl(PyObject *self)
 /*[clinic end generated code: output=63c6a6f57d783d6d input=f4494d5c31dbc6b7]*/
 {
     PyObject *res;
+    int closed = iobase_is_closed(self);
 
-    if (IS_CLOSED(self))
+    if (closed < 0) {
+        return NULL;
+    }
+    if (closed) {
         Py_RETURN_NONE;
+    }
 
     res = PyObject_CallMethodObjArgs(self, _PyIO_str_flush, NULL);
 
@@ -237,7 +273,7 @@ iobase_finalize(PyObject *self)
 
     /* If `closed` doesn't exist or can't be evaluated as bool, then the
        object is probably in an unusable state, so ignore. */
-    res = PyObject_GetAttr(self, _PyIO_str_closed);
+    res = _PyObject_GetAttrWithoutError(self, _PyIO_str_closed);
     if (res == NULL) {
         PyErr_Clear();
         closed = -1;
@@ -428,7 +464,7 @@ _PyIOBase_check_writable(PyObject *self, PyObject *args)
 static PyObject *
 iobase_enter(PyObject *self, PyObject *args)
 {
-    if (_PyIOBase_check_closed(self, Py_True) == NULL)
+    if (iobase_check_closed(self))
         return NULL;
 
     Py_INCREF(self);
@@ -472,7 +508,7 @@ static PyObject *
 _io__IOBase_isatty_impl(PyObject *self)
 /*[clinic end generated code: output=60cab77cede41cdd input=9ef76530d368458b]*/
 {
-    if (_PyIOBase_check_closed(self, Py_True) == NULL)
+    if (iobase_check_closed(self))
         return NULL;
     Py_RETURN_FALSE;
 }
@@ -499,24 +535,26 @@ _io__IOBase_readline_impl(PyObject *self, Py_ssize_t limit)
 {
     /* For backwards compatibility, a (slowish) readline(). */
 
-    int has_peek = 0;
-    PyObject *buffer, *result;
+    PyObject *peek, *buffer, *result;
     Py_ssize_t old_size = -1;
-    _Py_IDENTIFIER(peek);
 
-    if (_PyObject_HasAttrId(self, &PyId_peek))
-        has_peek = 1;
+    peek = _PyObject_GetAttrWithoutError(self, _PyIO_str_peek);
+    if (peek == NULL && PyErr_Occurred()) {
+        return NULL;
+    }
 
     buffer = PyByteArray_FromStringAndSize(NULL, 0);
-    if (buffer == NULL)
+    if (buffer == NULL) {
+        Py_XDECREF(peek);
         return NULL;
+    }
 
     while (limit < 0 || PyByteArray_GET_SIZE(buffer) < limit) {
         Py_ssize_t nreadahead = 1;
         PyObject *b;
 
-        if (has_peek) {
-            PyObject *readahead = _PyObject_CallMethodId(self, &PyId_peek, "i", 1);
+        if (peek != NULL) {
+            PyObject *readahead = PyObject_CallFunctionObjArgs(peek, _PyLong_One, NULL);
             if (readahead == NULL) {
                 /* NOTE: PyErr_SetFromErrno() calls PyErr_CheckSignals()
                    when EINTR occurs so we needn't do it ourselves. */
@@ -593,17 +631,19 @@ _io__IOBase_readline_impl(PyObject *self, Py_ssize_t limit)
 
     result = PyBytes_FromStringAndSize(PyByteArray_AS_STRING(buffer),
                                        PyByteArray_GET_SIZE(buffer));
+    Py_XDECREF(peek);
     Py_DECREF(buffer);
     return result;
   fail:
+    Py_XDECREF(peek);
     Py_DECREF(buffer);
     return NULL;
 }
 
 static PyObject *
 iobase_iter(PyObject *self)
 {
-    if (_PyIOBase_check_closed(self, Py_True) == NULL)
+    if (iobase_check_closed(self))
         return NULL;
 
     Py_INCREF(self);
@@ -716,7 +756,7 @@ _io__IOBase_writelines(PyObject *self, PyObject *lines)
 {
     PyObject *iter, *res;
 
-    if (_PyIOBase_check_closed(self, Py_True) == NULL)
+    if (iobase_check_closed(self))
         return NULL;
 
     iter = PyObject_GetIter(lines);

Modules/_io/textio.c

@@ -29,7 +29,6 @@ _Py_IDENTIFIER(mode);
 _Py_IDENTIFIER(name);
 _Py_IDENTIFIER(raw);
 _Py_IDENTIFIER(read);
-_Py_IDENTIFIER(read1);
 _Py_IDENTIFIER(readable);
 _Py_IDENTIFIER(replace);
 _Py_IDENTIFIER(reset);
@@ -1202,7 +1201,17 @@ _io_TextIOWrapper___init___impl(textio *self, PyObject *buffer,
         goto error;
     self->seekable = self->telling = r;
 
-    self->has_read1 = _PyObject_HasAttrId(buffer, &PyId_read1);
+    res = _PyObject_GetAttrWithoutError(buffer, _PyIO_str_read1);
+    if (res != NULL) {
+        Py_DECREF(res);
+        self->has_read1 = 1;
+    }
+    else if (!PyErr_Occurred()) {
+        self->has_read1 = 0;
+    }
+    else {
+        goto error;
+    }
 
     self->encoding_start_of_stream = 0;
     if (_textiowrapper_fix_encoder_state(self) < 0) {
@@ -3013,15 +3022,9 @@ textiowrapper_newlines_get(textio *self, void *context)
     CHECK_ATTACHED(self);
     if (self->decoder == NULL)
         Py_RETURN_NONE;
-    res = PyObject_GetAttr(self->decoder, _PyIO_str_newlines);
-    if (res == NULL) {
-        if (PyErr_ExceptionMatches(PyExc_AttributeError)) {
-            PyErr_Clear();
-            Py_RETURN_NONE;
-        }
-        else {
-            return NULL;
-        }
+    res = _PyObject_GetAttrWithoutError(self->decoder, _PyIO_str_newlines);
+    if (res == NULL && !PyErr_Occurred()) {
+        Py_RETURN_NONE;
     }
     return res;
 }