Issue1889394

classification
Title: UnicodeDerived's == is broken
Type: behaviour Severity: normal
Components: Core Versions: 2.5alpha1
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: cgroves Nosy List: cgroves, leosoto, pjenvey, tristan
Priority: normal Keywords:

Created on 2008-02-08.04:23:31 by tristanlk, last changed 2008-08-10.21:11:15 by leosoto.

Messages
msg2069 (view) Author: tristan (tristanlk) Date: 2008-02-08.04:23:31
>>> class Test(unicode):
...     pass
...
>>> a = Test('{1:1}')
>>> a == {1:1}
Traceback (innermost last):
  File "<console>", line 1, in ?
TypeError: unicode.__cmp__(x,y) requires y to be 'unicode', not a 'dict'

should return False.

i fixed this with the following patch: but i think it may be more of a work around than a fix.

Index: src/org/python/core/PyObject.java
===================================================================
--- src/org/python/core/PyObject.java   (revision 4112)
+++ src/org/python/core/PyObject.java   (working copy)
@@ -1253,7 +1253,7 @@
                 return res;
             return _cmpeq_unsafe(o) == 0 ? Py.True : Py.False;
         } catch (PyException e) {
-            if (Py.matchException(e, Py.AttributeError)) {
+            if (Py.matchException(e, Py.AttributeError) || Py.matchException(e, Py.TypeError)) {
                 return Py.False;
             }
             throw e;
msg2070 (view) Author: tristan (tristanlk) Date: 2008-02-08.04:44:51
Just realised that this seems to solve the problems with Py*Derived classes failing on == with other classes, but i'm not sure if this is hacky or not. I can't think of an example where classAobj == classBobj could ever be True except for when __eq__ is defined in the function, in which case, this fix doesn't affect that result.
msg2071 (view) Author: tristan (tristanlk) Date: 2008-02-08.06:42:33
After a bit more playing

with my patch:

>>> class A(unicode): pass
... 
>>> class B(unicode): pass
... 
>>> A() == B()
False

however, CPython returns True for this case.

After playing around for a bit, simply removing the toString() function from PyUnicodeDerived solves this problem.
msg3150 (view) Author: Tristan King (tristan) Date: 2008-04-13.02:32:42
There is also a problem with !=
here is an additional patch to fix it

--- a/jython/src/org/python/core/PyObject.java
+++ b/jython/src/org/python/core/PyObject.java
@@ -1264,6 +1264,11 @@ public class PyObject implements Serializable {
             if (res != null)
                 return res;
             return _cmpeq_unsafe(o) != 0 ? Py.True : Py.False;
+       } catch (PyException e) {
+            if (Py.matchException(e, Py.TypeError)) {
+                return Py.False;
+            }
+            throw e;
         } finally {
             delete_token(ts, token);
             ts.compareStateNesting--;
msg3155 (view) Author: Philip Jenvey (pjenvey) Date: 2008-04-15.01:29:27
2.2 handles this correctly -- it looks like a TypeError raised here is 
new -- the expose MethodType.CMP descriptor is throwing it.

'hi' == {} still works ok because _cmpeq_unsafe calls PyString __cmp__ 
directly (which returns -2) but a call to a derived's __cmp__ actually 
calls its exposed __cmp__, which is compiled to throw a TypeError when 
it gets back -2

We probably just want the exposed __cmp__ to not throw a TypeError here 
instead. Assigning over to Charlie because I think he might have changed 
it (the exposer is causing the TypeError)
msg3164 (view) Author: Leonardo Soto (leosoto) Date: 2008-04-21.21:11:29
If we want to behave exactly like CPython, __cmp__ should continue
throwing TypeError ({}.__cmp__('foo') does throws such exception there).

It seems that CPython does some trickery after calling __cmp__, but
doesn't apply it to every object, as shown on
>http://pylonshq.com/pasties/779>. And, if I understand correctly, the
current patch is going to swallow TypeError on every __cmp__ invocation,
unlike CPython.
msg3165 (view) Author: Leonardo Soto (leosoto) Date: 2008-04-21.21:12:32
Oh, sorry, the broken URL should be http://pylonshq.com/pasties/779
msg3168 (view) Author: Leonardo Soto (leosoto) Date: 2008-04-29.14:07:46
BTW, my comment apply to the dict.__cmp__ behavior. I didn't realized
that this issue was about unicode.__cmp__ behavior, although both may be
 related.
msg3330 (view) Author: Tristan King (tristan) Date: 2008-07-16.01:27:49
this seems to be fixed in 2.5a1+
msg3414 (view) Author: Leonardo Soto (leosoto) Date: 2008-08-10.21:11:15
Indeed. 

I've also added test cases for some of the snippets shown on this issue
on r5134. Thanks!
History
Date User Action Args
2008-08-10 21:11:15leosotosetstatus: open -> closed
resolution: fixed
messages: + msg3414
2008-07-16 01:27:49tristansetnosy: - tristanlk
messages: + msg3330
2008-04-29 14:07:48leosotosetmessages: + msg3168
2008-04-21 21:12:33leosotosetmessages: + msg3165
2008-04-21 21:11:29leosotosetnosy: + leosoto
messages: + msg3164
2008-04-15 01:29:27pjenveysetassignee: pjenvey -> cgroves
messages: + msg3155
nosy: + cgroves
2008-04-14 07:29:23pjenveysetassignee: pjenvey
nosy: + pjenvey
2008-04-13 02:32:43tristansettype: behaviour
messages: + msg3150
nosy: + tristan
versions: + 2.5alpha1
2008-02-08 04:23:31tristanlkcreate