Inefficient comparison code generation

[gfurnish]

Consider the following code:

                if left._name==(<SymbolicVariable_class>right)._name:

Which generates:

     __pyx_2 = PyObject_RichCompare(__pyx_v_left->__pyx_base._name, ((struct _\
_pyx_obj_4sage_9symbolics_14variable_class_SymbolicVariable_class *)__pyx_v_rig\
ht)->__pyx_base._name, Py_EQ); if (unlikely(!__pyx_2)) {__pyx_filename = __pyx_\
f[0]; __pyx_lineno = 126; __pyx_clineno = __LINE__; goto __pyx_L1;}
      __pyx_1 = __Pyx_PyObject_IsTrue(__pyx_2); if (unlikely(__pyx_1 < 0)) {__p\
yx_filename = __pyx_f[0]; __pyx_lineno = 126; __pyx_clineno = __LINE__; goto __\
pyx_L1;}

Is there a reason we don't use the

int PyObject_RichCompareBool(	PyObject *o1, PyObject *o2, int opid)

functionality in python to avoid the extra python boolean step?

[robertwb]

Rich comparisons may return things other than True or False. We would have to notice that the value is being used as a C boolean to allow calling RichCompareBool?.

If you look at the source, calling RichCompare? directly turns out to be faster unless a is b.

From Object/object.c

/* Return -1 if error; 1 if v op w; 0 if not (v op w). */
int
PyObject_RichCompareBool(PyObject *v, PyObject *w, int op)
{
	PyObject *res;
	int ok;

	/* Quick result when objects are the same.
	   Guarantees that identity implies equality. */
	if (v == w) {
		if (op == Py_EQ)
			return 1;
		else if (op == Py_NE)
			return 0;
	}

	res = PyObject_RichCompare(v, w, op);
	if (res == NULL)
		return -1;
	if (PyBool_Check(res))
		ok = (res == Py_True);
	else
		ok = PyObject_IsTrue(res);
	Py_DECREF(res);
	return ok;
}

[gfurnish]

However, "a is b" is the case for strings(the case in question here) and many other comparisons. Closing this as invalid does not seem appropriate (although changing the priority probably is).

[scoder]

I agree that closing this as "invalid" doesn't make the issue go away. It is not uncommon to have identical objects on both sides of an equals operator and a pointer comparison is so much faster than a call to PyObject_RichCompare() that it's worth optimising when the result of the comparison will be used as a boolean value.

However, a simple call to PyObject_RichCompareBool() is not completely identical with PyObject_RichCompare() + a boolean coercion, e.g. regarding internal typing in Cython and also regarding the runtime error handling code, so Cython would need to do a tiny bit more in order to make this optimisation both safe and correct.