[Python-Dev] RE: [Python-checkins] python/dist/src/Objects typeobject.c, 2.244, 2.245

[Guido van Rossum]

>   	if (res == -1 && PyErr_Occurred())
>   		return NULL;
> ! 	return PyInt_FromLong((long)res);
>   }
>   
> --- 3577,3583 ----
>   	if (res == -1 && PyErr_Occurred())
>   		return NULL;
> ! 	ret = PyObject_IsTrue(PyInt_FromLong((long)res)) ? Py_True :
> Py_False;
> 
> 
> The line above leaks and does unnecessary work. I believe it should
> read:
> 
>   	ret = res ? Py_True : Py_False;

Ai.  I did the review while only half awake. :-)

But the correct thing to do is to use PyBool_FromLong(res); there's
really no need to inline what that function does.

> Also, there is another one of these in Objects/descrobject.c line 712.

I'll fix that one while I'm at it.

BTW, I notice there are a bunch of uses of PyBool_FromLong() that are
preceded by something like "if (res < 0) return NULL;" (or "!= -1").

Maybe PyBool_FromLong() itself could make this unneeded by adding
something like

    if (ok < 0 && PyErr_Occurred())
        return NULL;

to its start?

And, while we're reviewing usage patterns of PyBool_FromLong(), the
string and unicode types are full of places where it is called by a
return statement with a constant 1 or 0 as argument.  This seems
wasteful to me; I imagine that

  Py_INCREF(Py_True);
  return Py_True;

takes less time than

  return PyBool_FromLong(1);

Maybe a pair of macros Py_return_True and Py_return_False would make
sense?

--Guido van Rossum (home page: http://www.python.org/~guido/)