[Python-bugs-list] [ python-Bugs-569257 ] __slots__ attribute and private variable
noreply@sourceforge.net
noreply@sourceforge.net
Thu, 20 Jun 2002 14:44:37 -0700
Bugs item #569257, was opened at 2002-06-14 20:54
You can respond by visiting:
http://sourceforge.net/tracker/?func=detail&atid=105470&aid=569257&group_id=5470
Category: Type/class unification
Group: Python 2.3
Status: Open
>Resolution: Accepted
Priority: 5
Submitted By: Keyton Weissinger (keytonw)
>Assigned to: Raymond Hettinger (rhettinger)
Summary: __slots__ attribute and private variable
Initial Comment:
If you use a __slots__ class variable and include a
variable with a double underscore as the first two
characters, that variable is NOT private as would be the
case for any other variable whose name begins with a
double underscore.
If this is by design, then it should be in the docs. If it is
not and represents a bug, it needs to be investigated.
Thank you!!!
Keyton Weissinger
----------------------------------------------------------------------
>Comment By: Guido van Rossum (gvanrossum)
Date: 2002-06-20 17:44
Message:
Logged In: YES
user_id=6380
Hm, I don't like the code duplication. :-(
I liked the previous version better.
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2002-06-20 17:38
Message:
Logged In: YES
user_id=80475
Rather than just check it in, I took another crack at it and
added a patch with the special (and most common) case
where the refcnt==1.
I haven't decided whether I like it. It doubles the code size,
but is fast, doesn't waste mallocs, has a clear
intention/expression, and it is easy to verify the refcnts.
Let me know whether you prefer this or the previous
version (with the null check added).
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2002-06-20 14:17
Message:
Logged In: YES
user_id=6380
Looks fine now, except you are missing an "if (newslots ==
NULL) return NULL;" error check.
I sort of wish that we could avoid first turning the list
(or other sequence) into a tuple and then making another
copy, but the full dance to turn something into a tuple is
complicated (see PySequence_Tuple -- it's got special cases
galore). Maybe you can avoid creating a new tuple when the
refcnt equals one -- then you know it's a new tuple returned
by PySequence_Tuple() that isn't shared with anyone else.
E.g.
if (slots->ob_refcnt == 1) {
Py_INCREF(slots);
newslots = slots;
}
else {
newslots = PyTuple_New(nslots);
}
But then the subsequent loop must be a bit more careful with
refcounts.
OK if you check in what you have now, which is more provably
correct (after adding the error check).
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2002-06-20 01:19
Message:
Logged In: YES
user_id=80475
Done.
Revised patch attached.
No more modifying immutables for me!
Thanks for the comments -- they really help me get to the
root of a problem, get it fixed, and learn something for the
next bugfix.
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2002-06-19 23:04
Message:
Logged In: YES
user_id=6380
Eh, no, I think there's a bug. The slots variable is made a
tuple by calling PySequence_Tuple(). If it was already a
tuple, this adds a reference to the existing tuple. And
later you're modifying it in-place! This gives the
"impossible" semantics of the following example:
>>> t = ("__foo", "__bar")
>>> class C(object):
__slots__ = t
>>> t
('_C__foo', '_C__bar')
>>>
I'm afraid you'll have to clone the tuple when you find its
refcount is >1.
Also, please make lines < 79 chars. And a readability
recommendation: if an if statement's expression is several
lines long, don't hide the '{' at the end of a line, but put
it at the beginning of the next, under the if: e.g.
if (blah, blah, blah,
blah, blah, blah,
blah, blah, blah)
{
blah;
blah;
}
Barry taught me this trick. :-)
Also, I think it's wrong to #include "compile.h" in
Python.h, it exposes too many internals... Better declare
the _Py_Munge() prototype somewhere else (directly in
Python.h would be fine).
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2002-06-19 22:51
Message:
Logged In: YES
user_id=80475
Done. New patch attached. Okay to commit?
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2002-06-19 15:30
Message:
Logged In: YES
user_id=6380
rhettinger: I haven't seen the patch, but I agree this ought
to be fixed -- in Python 2.3, as a new feature though. Why
don't you make munge() a private global (with an _Py prefix
to its name).
keytonw: I have no idea what you meant in your comment about
properties and slots and __property_name__. Can you
elaborate that?
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2002-06-19 14:13
Message:
Logged In: YES
user_id=80475
Added patch.
Note, the patch would be much shorter if mangle() in
compile.c were made non-static.
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2002-06-18 00:19
Message:
Logged In: YES
user_id=80475
Agree that it is a bug.
Working on a patch.
----------------------------------------------------------------------
Comment By: Keyton Weissinger (keytonw)
Date: 2002-06-14 23:31
Message:
Logged In: YES
user_id=396481
Also, this issue prevents you from using the combination of
property() and __slots__ which would be powerful, as
attempting to use them would make __property_name__ a
legal public variable, no?
----------------------------------------------------------------------
You can respond by visiting:
http://sourceforge.net/tracker/?func=detail&atid=105470&aid=569257&group_id=5470