Issue859555

Noting that the python code can call the top-level compare 
again after a coercion, I changed _cmp_unsafe to call _cmp 
again if the coercion did anything.

Also, python uses id() to order unordered classes.

Combined, these changes make test_compare.py pass.

Logged In: YES 
user_id=61408

I have looked at this:

1. comparisons and coercion are rather messy, it is hard to
distinguish what is language semantics at the moment vs.
hysterical raisins because of rich vs __cmp__ comparisions,
things still relying on coercion or not, the fact that
__coerce__ is only supported for old-style classes or by
coerce() builtin which OTOH is going to be phased out. See
PEPs 207,208 and the part about coercions in the Lang Ref.
2. Related: if you have noticed for __add__ etc we have
moved coercion logic insided the PyObject subclasses and
PyInstance, this is the spirit of PEP208, _cmp is the
exception, one thing
is that 3wy-comparision in CPython eagerly tries coercion in
both
direction, this is different for the binary ops.
[sadly btw as someone noticed the logic used by __add__ vs.
__coerce__ex__ are out of sync for some of our types]

3. This has prodded me to look seriously at comparision.

Consider this attached script.

Under python 2.2 the output is the following:
log: ['C_coerce'] coerce((),C()): (<__main__.D instance at
0x0076C0A8>, <__main_
_.D instance at 0x0076C070>)
log: ['C_coerce', 'C_coerce'] cmp(C(),C()): 1
log: ['C_coerce', 'D_eq'] cmp(C(),D()): 0
log: ['C_coerce', 'D_eq'] cmp(D(),C()): 0
log: ['D_eq'] cmp(D(),D()): 0
log: ['C_coerce'] C()==C(): 0
log: ['D_eq'] C()==D(): 1
log: ['D_eq'] D()==C(): 1
log: ['D_eq'] D()==D(): 1

A) coercing is attempted eagerly, this is different from out
__class__ comparison logic
B) the first C_coerce  in the logs for the cmp are because
of these lines in do_cmp in CPython 2.2 object.c:
	if (v->ob_type == w->ob_type
	    && (f = v->ob_type->tp_compare) != NULL) {
		c = (*f)(v, w);
		if (c != 2 || !PyInstance_Check(v))
			return c;
	}
and  that instance_compare in classobject.c try coercion
eagerly.
....

Logged In: YES 
user_id=61408

...
that code means that if it's there, cmp will try __cmp__ before
rich comparisons,  at the moment we don't have support
to naturally check something like v->ob_type->tp_compare !=
NULL, I expect we will grow it with new-style classes code.
We could also punt on this.

Notice that eagerly coercing cmp(C(),C()) does not try D's
rich __eq__.

the patch like the previous code does not eagerly coerce,
this is the output with a patched jython:

log: [] coerce((),C()): (<__main__.C instance 1>,
<__main__.C instance 2>)
log: [] cmp(C(),C()): -1
log: ['D_eq'] cmp(C(),D()): 0
log: ['D_eq'] cmp(D(),C()): 0
log: ['D_eq'] cmp(D(),D()): 0
log: [] C()==C(): 0
log: ['D_eq'] C()==D(): 1
log: ['D_eq'] D()==C(): 1
log: ['D_eq'] D()==D(): 1

....

Logged In: YES 
user_id=61408

if we eagerly coerce then (at least for instances), for
example changing half_cmp to test:

        if (o1.__class__ != o2.__class__ || o1 instanceof
PyInstance
            || o2 instanceof PyInstance) {

the output becomes:
log: [] coerce((),C()): (<__main__.C instance 1>,
<__main__.C instance 2>)
log: ['C_coerce', 'D_eq'] cmp(C(),C()): 0
log: ['D_eq'] cmp(C(),D()): 0
log: ['D_eq'] cmp(D(),C()): 0
log: ['D_eq'] cmp(D(),D()): 0
log: ['C_coerce', 'D_eq'] C()==C(): 1
log: ['D_eq'] C()==D(): 1
log: ['D_eq'] D()==C(): 1
log: ['D_eq'] D()==D(): 1

[yep, also builtin coerce need fixing]

notice the 0 because 
  return o1._cmp(o2); 
tries __eq__.

I'm tempted to go with something like this [not tested]:

        int half = this.__cmp__(other)
        if (half != -2)
        {
            return half;
        }
        if (!(this instanceof PyInstance) {
           half = other.__cmp__(this);
          if (half != -2)
          {
              return -half;
          }
        }

pushing the coercion logic down the PyObject subclasses like
we do for all the other bin ops, and having
PyInstance.__cmp__ doing everything necessary for that case
like instance_compare
in CPython does. I may try this on the newstyle branch.

Thanks.

Logged In: YES 
user_id=920303

I agree that in moving to the newstyle stuff, making the code 
more like the CPython code is saner.

Fair enough

Logged In: YES 
user_id=61408

I indeed attacked this slitghly differently on the new-style
branch, there test_compare and test_richcmp too (if I recall
correctly) also work.

Logged In: YES 
user_id=37674

The test cases were fixed during the new-style conversion.