tp_(get|set)attro? inheritance bug

[gustavo]

BPO	975646
Nosy	@mwhudson, @gvanrossum, @loewis, @devdanzin

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2009-02-14.16:19:08.785>
created_at = <Date 2004-06-18.22:04:18.000>
labels = ['interpreter-core', 'type-bug']
title = 'tp_(get|set)attro? inheritance bug'
updated_at = <Date 2009-02-14.16:19:08.784>
user = 'https://bugs.python.org/gustavo'

bugs.python.org fields:

activity = <Date 2009-02-14.16:19:08.784>
actor = 'benjamin.peterson'
assignee = 'none'
closed = True
closed_date = <Date 2009-02-14.16:19:08.785>
closer = 'benjamin.peterson'
components = ['Interpreter Core']
creation = <Date 2004-06-18.22:04:18.000>
creator = 'gustavo'
dependencies = []
files = []
hgrepos = []
issue_num = 975646
keywords = []
message_count = 7.0
messages = ['21219', '21220', '21221', '21222', '21223', '82059', '82061']
nosy_count = 5.0
nosy_names = ['mwh', 'gvanrossum', 'loewis', 'gustavo', 'ajaksu2']
pr_nums = []
priority = 'normal'
resolution = 'wont fix'
stage = None
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue975646'
versions = ['Python 2.6']

[gustavo]

Documentation says, regarding tp_getattr:
«
This field is inherited by subtypes together with
tp_getattro: a subtype inherits both tp_getattr and
tp_getattro from its base type when the subtype's
tp_getattr and tp_getattro are both NULL.
»

Implementation disagrees, at least in cvs head, but
the effect of the bug (non-inheritance of tp_getattr)
happens in 2.3.3. Follow with me:

In function type_new (typeobject.c) line 1927:
        /* Special case some slots */
        if (type->tp_dictoffset != 0 || nslots > 0) {
                if (base->tp_getattr == NULL &&
base->tp_getattro == NULL)
                        type->tp_getattro =
PyObject_GenericGetAttr;
                if (base->tp_setattr == NULL &&
base->tp_setattro == NULL)
                        type->tp_setattro =
PyObject_GenericSetAttr;
        }
...later in the same function...

        /* Initialize the rest */
        if (PyType_Ready(type) < 0) {
                Py_DECREF(type);
                return NULL;
        }

Inside PyType_Ready(), line 3208:
        for (i = 1; i < n; i++) {
                PyObject *b = PyTuple_GET_ITEM(bases, i);
                if (PyType_Check(b))
                        inherit_slots(type,
(PyTypeObject *)b);
        }

Inside inherit_slots, line (3056):
if (type->tp_getattr == NULL &&
type->tp_getattro == NULL) {
type->tp_getattr = base->tp_getattr;
type->tp_getattro = base->tp_getattro;
}
if (type->tp_setattr == NULL &&
type->tp_setattro == NULL) {
type->tp_setattr = base->tp_setattr;
type->tp_setattro = base->tp_setattro;
}

So, if you have followed through, you'll notice

that type_new first sets tp_getattro = GenericGetAttr,
in case 'base' has neither tp_getattr nor tp_getattro.

So, you are thinking that there is no problem.  If

base has tp_getattr, that code path won't be execute.
The problem is with multiple inheritance. In type_new,
'base' is determined by calling best_base(). But the
selected base may not have tp_getattr, while
another might have. In this case, setting tp_getattro
based on information from the wrong base precludes the
slot from being inherited from the right base. This is
happening in pygtk, unfortunately.

One possible solution would be to move the first

code block to after the PyType_Ready() call.

[loewis]

Logged In: YES
user_id=21627

Guido, is this a bug?

[mwhudson]

Logged In: YES
user_id=6656

Without wanting to think about this terribly hard, wouldn't a
workaround be for pygtk to implement tp_getattro and not
tp_getattr? IMO, this is a good idea anyway...

[gvanrossum]

Logged In: YES
user_id=6380

No time to think about this more, but yes, a type should
only implement *one* of tp_getattr and tp_getattro, and only
*one* (best the matching one!) of tp_setattr and
tp_setattro. If you are trying to get different semantics
for getattro and getattr, you're crazy (that's not what the
two are for -- tp_getattr is only there for b/w compat).

[gustavo]

Logged In: YES
user_id=908

PyGTK implements only tp_getattr, none of tp_getattro,
tp_setattr, or tp_setattro.

We already have a workaround in pygtk, but it was my civic
duty to report this bug O:-)

[devdanzin]

Not a bug, then? Will close unless someone argues against it.

[gustavo]

If this problem does not apply to Python 3.x, by all means go ahead and
close it. It probably is a bug, but probably not worth fixing;
workaround is simple enough, and affected extensions have already
adapted to it.