Code review comment for lp:~jameinel/bzr/2.1-simple-set

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

...

> Again, PyObject_IsTrue tries == Py_True as the very first thing, so the cost
> should be small. If you're worried a macro along the lines of:
>
> #define PY_IS_TRUE(o) ((o == Py_True) || PyObject_IsTrue(o))
>
> might make us both happy.
>

So it turns out that this fix actually causes crazy corruption. Specifically

assert PyObject_IsTrue(Py_NotImplemented)

passes.

In other words "Py_NotImplemented" evaluates to True.
You can also see that with:

>>> bool(NotImplemented)
True

So the code was doing:

if res == NULL:
  return -1
if PyObject_IsTrue(res):
  ...
if res == Py_NotImplemented:
  # reverse the comparison.

Anyway, I've tracked this down to some crazy interning issues I've been
seeing. (You have to get a hash collision *and* have objects that return
NotImplemented.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkrV72AACgkQJdeBCYSNAAPiOgCdGOnxjBvPH08fKYxa0vjXHSGu
yfsAn0Mn8b62R8wn8jHsttXoBZEILhja
=3Wq/
-----END PGP SIGNATURE-----