[Python-Dev] [Python-checkins] cpython (3.2): don't use a slot wrapper from a different special method (closes #14658)

[Mark Shannon]

I'm not happy with this fix.

Admittedly code like:

class S(str):
    __getattr__ = str.__add__
s = S('a')
print(S.b)

is a little weird.
But I think it should work (ie print 'ab') properly.

This works without the patch.

class S(str):
    __getattribute__ = str.__add__
s = S('a')
print(S.b)

(Prints 'ab')

Also "slot wrapper" is a low-level implementation detail and
shouldn't impact the language semantics.

dict.__getitem__ is a slot wrapper; dict.__getitem__ is not.
str.__getitem__ is a slot wrapper; list.__getitem__ is not.
If any of these change then the semantics of the language changes.

Cheers,
Mark

benjamin.peterson wrote:
> http://hg.python.org/cpython/rev/971865f12377
> changeset:   76518:971865f12377
> branch:      3.2
> parent:      76506:f7b002e5cac7
> user:        Benjamin Peterson <benjamin at python.org>
> date:        Tue Apr 24 11:06:25 2012 -0400
> summary:
>   don't use a slot wrapper from a different special method (closes #14658)
> 
> This also alters the fix to #11603. Specifically, setting __repr__ to
> object.__str__ now raises a recursion RuntimeError when str() or repr() is
> called instead of silently bypassing the recursion. I believe this behavior is
> more correct.
> 
> files:
>   Lib/test/test_descr.py |  10 +++++++++-
>   Misc/NEWS              |   6 ++++++
>   Objects/typeobject.c   |   5 +++--
>   3 files changed, 18 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py
> --- a/Lib/test/test_descr.py
> +++ b/Lib/test/test_descr.py
> @@ -4430,7 +4430,15 @@
>              pass
>          Foo.__repr__ = Foo.__str__
>          foo = Foo()
> -        str(foo)
> +        self.assertRaises(RuntimeError, str, foo)
> +        self.assertRaises(RuntimeError, repr, foo)
> +
> +    def test_mixing_slot_wrappers(self):
> +        class X(dict):
> +            __setattr__ = dict.__setitem__
> +        x = X()
> +        x.y = 42
> +        self.assertEqual(x["y"], 42)
>  
>      def test_cycle_through_dict(self):
>          # See bug #1469629
> diff --git a/Misc/NEWS b/Misc/NEWS
> --- a/Misc/NEWS
> +++ b/Misc/NEWS
> @@ -10,6 +10,12 @@
>  Core and Builtins
>  -----------------
>  
> +- Issue #11603 (again): Setting __repr__ to __str__ now raises a RuntimeError
> +  when repr() or str() is called on such an object.
> +
> +- Issue #14658: Fix binding a special method to a builtin implementation of a
> +  special method with a different name.
> +
>  - Issue #14630: Fix a memory access bug for instances of a subclass of int
>    with value 0.
>  
> diff --git a/Objects/typeobject.c b/Objects/typeobject.c
> --- a/Objects/typeobject.c
> +++ b/Objects/typeobject.c
> @@ -2928,7 +2928,7 @@
>      unaryfunc f;
>  
>      f = Py_TYPE(self)->tp_repr;
> -    if (f == NULL || f == object_str)
> +    if (f == NULL)
>          f = object_repr;
>      return f(self);
>  }
> @@ -5757,7 +5757,8 @@
>              }
>              continue;
>          }
> -        if (Py_TYPE(descr) == &PyWrapperDescr_Type) {
> +        if (Py_TYPE(descr) == &PyWrapperDescr_Type &&
> +            ((PyWrapperDescrObject *)descr)->d_base->name_strobj == p->name_strobj) {
>              void **tptr = resolve_slotdups(type, p->name_strobj);
>              if (tptr == NULL || tptr == ptr)
>                  generic = p->function;
> 
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Python-checkins mailing list
> Python-checkins at python.org
> http://mail.python.org/mailman/listinfo/python-checkins