Re: [Python-Dev] NotImplemented reaching top-level

2005-12-29 Thread M.-A. Lemburg
Hi Armin,

> On Wed, Dec 28, 2005 at 09:56:43PM +0100, M.-A. Lemburg wrote:
>>> d += 1.2
>>> d
 NotImplemented
>> The PEP documenting the coercion logic has complete tables
>> for what should happen:
> 
> Well, '+=' does not invoke coercion at all, with new-style classes like
> Decimal.

True, it doesn't invoke coercion in the sense that a coercion
method is called, but the mechanism described in the PEP is
still used via PyNumber_InPlaceAdd().

>> Looking at the code in abstract.c the above problem appears
>> to be related to the special cases applied to += and *=
>> in case both operands cannot deal with the type combination.
>>
>> In such a case, a check is done whether the operation could
>> be interpreted as sequence operation (concat or repeat) and
>> then delegated to the appropriate handlers.
> 
> Indeed.  The bug was caused by this delegation, which (prior to my
> patch) would also return a Py_NotImplemented that would leak through
> abstract.c.  My patch is to remove this unnecessary delegation by not
> defining sq_concat/sq_repeat for user-defined classes, and restoring the
> original expectation that the sq_concat/sq_repeat slots should not
> return Py_NotImplemented.  How does this relate to coercion?

The Py_NotImplemented singleton was introduced in the coercion
proposal to mean "there is no implementation to execute the requested
operation on the given combination of types".

At the time we also considered using an exception for this, but
it turned out that this caused too much of a slow-down. Hence the use
of a special singleton which could be tested for by a simple
pointer comparison.

Originally, the singleton was only needed for mixed-type operations.
It seems that its use has spread to other areas as well and
can now also refer to missing same-type operator implementations.

>> But then again, looking in typeobject.c, the following code
>> could be the cause for leaking a NotImplemented singleton
>> reference:
>>
>> #define SLOT1BINFULL(FUNCNAME, TESTFUNC, SLOTNAME, OPSTR, ROPSTR) \
>> static PyObject * \
>> FUNCNAME(PyObject *self, PyObject *other) \
>> { \
>>  static PyObject *cache_str, *rcache_str; \
>>  int do_other = self->ob_type != other->ob_type && \
>>  other->ob_type->tp_as_number != NULL && \
>>  other->ob_type->tp_as_number->SLOTNAME == TESTFUNC; \
>>  if (self->ob_type->tp_as_number != NULL && \
>>  self->ob_type->tp_as_number->SLOTNAME == TESTFUNC) { \
>>  PyObject *r; \
>>  if (do_other && \
>>  PyType_IsSubtype(other->ob_type, self->ob_type) && \
>>  method_is_overloaded(self, other, ROPSTR)) { \
>>  r = call_maybe( \
>>  other, ROPSTR, &rcache_str, "(O)", self); \
>>  if (r != Py_NotImplemented) \
>>  return r; \
>>  Py_DECREF(r); \
>>  do_other = 0; \
>>  } \
>>  r = call_maybe( \
>>  self, OPSTR, &cache_str, "(O)", other); \
>>  if (r != Py_NotImplemented || \
>>  other->ob_type == self->ob_type) \
>> ^
>> If both types are of the same type, then a NotImplemented returng
>> value would be returned.
> 
> Indeed, however:
> 
>>  return r; \
>>  Py_DECREF(r); \
>>  } \
>>  if (do_other) { \
>>  return call_maybe( \
>>  other, ROPSTR, &rcache_str, "(O)", self); \
>>  } \
>>  Py_INCREF(Py_NotImplemented); \
>>  return Py_NotImplemented; \
>> }
> 
> This last statement also returns Py_NotImplemented.  So it's expected of
> this function to be able to return Py_NotImplemented, isn't it?  The
> type slots like nb_add can return Py_NotImplemented; the code that
> converts it to a TypeError is in the caller, which is abstract.c.

You're right - silly me.

Regards,
-- 
Marc-Andre Lemburg
eGenix.com

Professional Python Services directly from the Source  (#1, Dec 29 2005)
>>> Python/Zope Consulting and Support ...http://www.egenix.com/
>>> mxODBC.Zope.Database.Adapter ... http://zope.egenix.com/
>>> mxODBC, mxDateTime, mxTextTools ...http://python.egenix.com/


::: Try mxODBC.Zope.DA for Windows,Linux,Solaris,FreeBSD for free ! 
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] NotImplemented reaching top-level

2005-12-28 Thread Armin Rigo
Hi Marc,

On Wed, Dec 28, 2005 at 09:56:43PM +0100, M.-A. Lemburg wrote:
> > d += 1.2
> > d
> >> NotImplemented
> 
> The PEP documenting the coercion logic has complete tables
> for what should happen:

Well, '+=' does not invoke coercion at all, with new-style classes like
Decimal.

> Looking at the code in abstract.c the above problem appears
> to be related to the special cases applied to += and *=
> in case both operands cannot deal with the type combination.
>
> In such a case, a check is done whether the operation could
> be interpreted as sequence operation (concat or repeat) and
> then delegated to the appropriate handlers.

Indeed.  The bug was caused by this delegation, which (prior to my
patch) would also return a Py_NotImplemented that would leak through
abstract.c.  My patch is to remove this unnecessary delegation by not
defining sq_concat/sq_repeat for user-defined classes, and restoring the
original expectation that the sq_concat/sq_repeat slots should not
return Py_NotImplemented.  How does this relate to coercion?

> But then again, looking in typeobject.c, the following code
> could be the cause for leaking a NotImplemented singleton
> reference:
> 
> #define SLOT1BINFULL(FUNCNAME, TESTFUNC, SLOTNAME, OPSTR, ROPSTR) \
> static PyObject * \
> FUNCNAME(PyObject *self, PyObject *other) \
> { \
>   static PyObject *cache_str, *rcache_str; \
>   int do_other = self->ob_type != other->ob_type && \
>   other->ob_type->tp_as_number != NULL && \
>   other->ob_type->tp_as_number->SLOTNAME == TESTFUNC; \
>   if (self->ob_type->tp_as_number != NULL && \
>   self->ob_type->tp_as_number->SLOTNAME == TESTFUNC) { \
>   PyObject *r; \
>   if (do_other && \
>   PyType_IsSubtype(other->ob_type, self->ob_type) && \
>   method_is_overloaded(self, other, ROPSTR)) { \
>   r = call_maybe( \
>   other, ROPSTR, &rcache_str, "(O)", self); \
>   if (r != Py_NotImplemented) \
>   return r; \
>   Py_DECREF(r); \
>   do_other = 0; \
>   } \
>   r = call_maybe( \
>   self, OPSTR, &cache_str, "(O)", other); \
>   if (r != Py_NotImplemented || \
>   other->ob_type == self->ob_type) \
> ^
> If both types are of the same type, then a NotImplemented returng
> value would be returned.

Indeed, however:

> 
>   return r; \
>   Py_DECREF(r); \
>   } \
>   if (do_other) { \
>   return call_maybe( \
>   other, ROPSTR, &rcache_str, "(O)", self); \
>   } \
>   Py_INCREF(Py_NotImplemented); \
>   return Py_NotImplemented; \
> }

This last statement also returns Py_NotImplemented.  So it's expected of
this function to be able to return Py_NotImplemented, isn't it?  The
type slots like nb_add can return Py_NotImplemented; the code that
converts it to a TypeError is in the caller, which is abstract.c.


A bientot,

Armin
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] NotImplemented reaching top-level

2005-12-28 Thread M.-A. Lemburg
Armin Rigo wrote:
> Hi Facundo,
> 
> On Sat, Dec 24, 2005 at 02:31:19PM -0300, Facundo Batista wrote:
> d += 1.2
> d
>> NotImplemented
> 
> The situation appears to be a mess.  Some combinations of specific
> operators fail to convert NotImplemented to a TypeError, depending on
> old- or new-style-class-ness, although this is clearly a bug (e.g. in an
> example like yours but using -= instead of +=, we get the correct
> TypeError.)
> 
> Obviously, we need to write some comprehensive tests about this.  But
> now I just found out that the old, still-pending SF bug #847024 about
> A()*5 in new-style classes hasn't been given any attention; my theory is
> that nobody fully understands the convoluted code paths of abstract.c
> any more :-(

The PEP documenting the coercion logic has complete tables
for what should happen:

http://www.python.org/peps/pep-0208.html

Looking at the code in abstract.c the above problem appears
to be related to the special cases applied to += and *=
in case both operands cannot deal with the type combination.

In such a case, a check is done whether the operation could
be interpreted as sequence operation (concat or repeat) and
then delegated to the appropriate handlers.

But then again, looking in typeobject.c, the following code
could be the cause for leaking a NotImplemented singleton
reference:

#define SLOT1BINFULL(FUNCNAME, TESTFUNC, SLOTNAME, OPSTR, ROPSTR) \
static PyObject * \
FUNCNAME(PyObject *self, PyObject *other) \
{ \
static PyObject *cache_str, *rcache_str; \
int do_other = self->ob_type != other->ob_type && \
other->ob_type->tp_as_number != NULL && \
other->ob_type->tp_as_number->SLOTNAME == TESTFUNC; \
if (self->ob_type->tp_as_number != NULL && \
self->ob_type->tp_as_number->SLOTNAME == TESTFUNC) { \
PyObject *r; \
if (do_other && \
PyType_IsSubtype(other->ob_type, self->ob_type) && \
method_is_overloaded(self, other, ROPSTR)) { \
r = call_maybe( \
other, ROPSTR, &rcache_str, "(O)", self); \
if (r != Py_NotImplemented) \
return r; \
Py_DECREF(r); \
do_other = 0; \
} \
r = call_maybe( \
self, OPSTR, &cache_str, "(O)", other); \
if (r != Py_NotImplemented || \
other->ob_type == self->ob_type) \
^
If both types are of the same type, then a NotImplemented returng
value would be returned.

return r; \
Py_DECREF(r); \
} \
if (do_other) { \
return call_maybe( \
other, ROPSTR, &rcache_str, "(O)", self); \
} \
Py_INCREF(Py_NotImplemented); \
return Py_NotImplemented; \
}

Strange enough, the SLOT1BINFULL macro is not used by the
inplace concat or repeat slots...

-- 
Marc-Andre Lemburg
eGenix.com

Professional Python Services directly from the Source  (#1, Dec 28 2005)
>>> Python/Zope Consulting and Support ...http://www.egenix.com/
>>> mxODBC.Zope.Database.Adapter ... http://zope.egenix.com/
>>> mxODBC, mxDateTime, mxTextTools ...http://python.egenix.com/


::: Try mxODBC.Zope.DA for Windows,Linux,Solaris,FreeBSD for free ! 
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] NotImplemented reaching top-level

2005-12-27 Thread Armin Rigo
Hi Facundo,

On Mon, Dec 26, 2005 at 02:31:10PM -0300, Facundo Batista wrote:
> > nb_add and nb_multiply should be tried.  I don't think that this would
> > break existing C or Python code, but it should probably only go in 2.5,
> > together with the patch #1390657 that relies on it.
> 
> It'd be good to know if this will be applied for the next version
> 2.4.x or will wait until 2.4.5, for me to search a workaround in
> Decimal or not (really don't know if I can find a solution here).

I completed the patch on the SF tracker, and now I believe that it could
safely be checked in the HEAD and in the 2.4 branch (after the
appropriate review).


A bientot,

Armin
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] NotImplemented reaching top-level

2005-12-26 Thread Facundo Batista
2005/12/26, Armin Rigo <[EMAIL PROTECTED]>:

> Done in patch #1390657.

Fantastic, Armin, thank you!


> nb_add and nb_multiply should be tried.  I don't think that this would
> break existing C or Python code, but it should probably only go in 2.5,
> together with the patch #1390657 that relies on it.

It'd be good to know if this will be applied for the next version
2.4.x or will wait until 2.4.5, for me to search a workaround in
Decimal or not (really don't know if I can find a solution here).

Thank you!

.Facundo

Blog: http://www.taniquetil.com.ar/plog/
PyAr: http://www.python.org/ar/
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] NotImplemented reaching top-level

2005-12-26 Thread Armin Rigo
Hi,

On Mon, Dec 26, 2005 at 02:40:38AM +1000, Nick Coghlan wrote:
> That sounds like the right definition to me (I believe this behaviour is what 
> Raymond and Facundo were aiming for with the last round of updates to 
> Decimal).

Done in patch #1390657.

Although this patch passes all existing tests plus the ones it adds,
there is a corner and untested case where it could potentially break
code.  Indeed, the only sane patch I could come up with makes
user-defined types fail to work with PySequence_Concat() and
PySequence_Repeat() -- details in the patch.  So I propose that we
clarify what these two functions really mean in term of the Python
language spec, instead of just in term of the CPython-specific sq_concat
and sq_repeat slots.  (BTW that's needed for PyPy/Jython/etc., too, to
give a reasonable meaning to the operator.concat() and operator.repeat()
built-ins.)

I propose the following definitions (which are mostly what the
docstrings already explain anyway):

* PySequence_Concat(a, b) and operator.concat(a, b) mean "a + b", with
  the difference that we check that both arguments appear to be
  sequences (as checked with operator.isSequenceType()).

* PySequence_Repeat(a, b) and operator.repeat(a, b) mean "a * b", where
  "a" is checked to be a sequence and "b" is an integer.  Some bounds
  can be enforced on "b" -- for CPython, it means that it must fit in a
  C int.

The idea is to extend PySequence_Concat() and PySequence_Repeat() to
match the above definitions precisely, which means that for objects not
defining sq_repeat or sq_concat but still appearing to be sequences,
nb_add and nb_multiply should be tried.  I don't think that this would
break existing C or Python code, but it should probably only go in 2.5,
together with the patch #1390657 that relies on it.


A bientot,

Armin
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] NotImplemented reaching top-level

2005-12-26 Thread Armin Rigo
Hi Brett,

On Sun, Dec 25, 2005 at 11:55:11AM -0800, Brett Cannon wrote:
> Maybe.  Also realize we will have a chance to clean it up when Python
> 3 comes around since the classic class stuff will be ripped out.  That
> way we might have a chance to streamline the code.

For once, old-style classes are not to blame here: it's only about the
oldest aspects of the PyTypeObject structure and substructures.


I-said-that-no-one-knows-this-code-any-more'ly yours,

Armin
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] NotImplemented reaching top-level

2005-12-26 Thread Neil Schemenauer
Armin Rigo <[EMAIL PROTECTED]> wrote:
> Of course, speaking of a rewrite, PyPy does the "right thing" in
> these two areas.  Won't happen to CPython, though.  There are too
> much backward-compatibility issues with the PyTypeObject
> structure; I think we're doomed with patching the bugs as they
> show up.

This is definitely something that should be cleaned up for Python
3k.

> Looking up in the language reference, I see no mention of NotImplemented
> in the page about __add__, __radd__, etc.  I guess it's a documentation
> bug as well, isn't it?  The current code base tries to implement the
> following behavior: Returning NotImplemented from any of the binary
> special methods (__xxx__, __rxxx__, __ixxx__) makes Python proceed as if
> the method was not defined in the first place.
>
> If we agree on this, I could propose a doc fix, a test, and appropriate
> bug fixes.

I believe that's correct.

  Neil

___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] NotImplemented reaching top-level

2005-12-25 Thread Brett Cannon
On 12/25/05, Armin Rigo <[EMAIL PROTECTED]> wrote:
> Hi Reinhold,
>
> On Sun, Dec 25, 2005 at 12:37:53PM +0100, Reinhold Birkenfeld wrote:
> > > that nobody fully understands the convoluted code paths of abstract.c
> > > any more :-(
> >
> > Time for a rewrite?
>

Maybe.  Also realize we will have a chance to clean it up when Python
3 comes around since the classic class stuff will be ripped out.  That
way we might have a chance to streamline the code.

> Of course, speaking of a rewrite, PyPy does the "right thing" in these
> two areas.  Won't happen to CPython, though.  There are too much
> backward-compatibility issues with the PyTypeObject structure; I think
> we're doomed with patching the bugs as they show up.
>
> Looking up in the language reference, I see no mention of NotImplemented
> in the page about __add__, __radd__, etc.  I guess it's a documentation
> bug as well, isn't it?  The current code base tries to implement the
> following behavior: Returning NotImplemented from any of the binary
> special methods (__xxx__, __rxxx__, __ixxx__) makes Python proceed as if
> the method was not defined in the first place.
>

This is what I always assumed the behaviour was supposed to be, so I
am quite happy to go with that.

-Brett
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] NotImplemented reaching top-level

2005-12-25 Thread Nick Coghlan
Armin Rigo wrote:
> Hi Reinhold,
> 
> On Sun, Dec 25, 2005 at 12:37:53PM +0100, Reinhold Birkenfeld wrote:
>>> that nobody fully understands the convoluted code paths of abstract.c
>>> any more :-(
>> Time for a rewrite?
> 
> Of course, speaking of a rewrite, PyPy does the "right thing" in these
> two areas.  Won't happen to CPython, though.  There are too much
> backward-compatibility issues with the PyTypeObject structure; I think
> we're doomed with patching the bugs as they show up.
> 
> Looking up in the language reference, I see no mention of NotImplemented
> in the page about __add__, __radd__, etc.  I guess it's a documentation
> bug as well, isn't it?  The current code base tries to implement the
> following behavior: Returning NotImplemented from any of the binary
> special methods (__xxx__, __rxxx__, __ixxx__) makes Python proceed as if
> the method was not defined in the first place.
> 
> If we agree on this, I could propose a doc fix, a test, and appropriate
> bug fixes.

That sounds like the right definition to me (I believe this behaviour is what 
Raymond and Facundo were aiming for with the last round of updates to Decimal).

Regards,
Nick.

-- 
Nick Coghlan   |   [EMAIL PROTECTED]   |   Brisbane, Australia
---
 http://www.boredomandlaziness.org
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] NotImplemented reaching top-level

2005-12-25 Thread Armin Rigo
Hi Reinhold,

On Sun, Dec 25, 2005 at 12:37:53PM +0100, Reinhold Birkenfeld wrote:
> > that nobody fully understands the convoluted code paths of abstract.c
> > any more :-(
> 
> Time for a rewrite?

Of course, speaking of a rewrite, PyPy does the "right thing" in these
two areas.  Won't happen to CPython, though.  There are too much
backward-compatibility issues with the PyTypeObject structure; I think
we're doomed with patching the bugs as they show up.

Looking up in the language reference, I see no mention of NotImplemented
in the page about __add__, __radd__, etc.  I guess it's a documentation
bug as well, isn't it?  The current code base tries to implement the
following behavior: Returning NotImplemented from any of the binary
special methods (__xxx__, __rxxx__, __ixxx__) makes Python proceed as if
the method was not defined in the first place.

If we agree on this, I could propose a doc fix, a test, and appropriate
bug fixes.


A bientot,

Armin
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] NotImplemented reaching top-level

2005-12-25 Thread Reinhold Birkenfeld
Armin Rigo wrote:
> Hi Facundo,
> 
> On Sat, Dec 24, 2005 at 02:31:19PM -0300, Facundo Batista wrote:
>> >>> d += 1.2
>> >>> d
>> NotImplemented
> 
> The situation appears to be a mess.  Some combinations of specific
> operators fail to convert NotImplemented to a TypeError, depending on
> old- or new-style-class-ness, although this is clearly a bug (e.g. in an
> example like yours but using -= instead of +=, we get the correct
> TypeError.)
> 
> Obviously, we need to write some comprehensive tests about this.  But
> now I just found out that the old, still-pending SF bug #847024 about
> A()*5 in new-style classes hasn't been given any attention; my theory is
> that nobody fully understands the convoluted code paths of abstract.c
> any more :-(

Time for a rewrite?

Reinhold

-- 
Mail address is perfectly valid!

___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] NotImplemented reaching top-level

2005-12-25 Thread Armin Rigo
Hi Facundo,

On Sat, Dec 24, 2005 at 02:31:19PM -0300, Facundo Batista wrote:
> >>> d += 1.2
> >>> d
> NotImplemented

The situation appears to be a mess.  Some combinations of specific
operators fail to convert NotImplemented to a TypeError, depending on
old- or new-style-class-ness, although this is clearly a bug (e.g. in an
example like yours but using -= instead of +=, we get the correct
TypeError.)

Obviously, we need to write some comprehensive tests about this.  But
now I just found out that the old, still-pending SF bug #847024 about
A()*5 in new-style classes hasn't been given any attention; my theory is
that nobody fully understands the convoluted code paths of abstract.c
any more :-(


A bientot,

Armin
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com