This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: __slots__ attribute and private variable
Type: Stage:
Components: Interpreter Core Versions: Python 2.3
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: gvanrossum, keytonw, rhettinger
Priority: normal Keywords:

Created on 2002-06-15 00:54 by keytonw, last changed 2022-04-10 16:05 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
slots.diff rhettinger, 2002-06-19 18:13 Slots patch.
slots2.diff rhettinger, 2002-06-20 02:51 New patch using _Py_Mangle()
slots3.diff rhettinger, 2002-06-20 05:19 New patch with cloned tuple
slots4.diff rhettinger, 2002-06-20 21:38 New patch with refcnt==1 special case
Messages (12)
msg11198 - (view) Author: Keyton Weissinger (keytonw) Date: 2002-06-15 00:54
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
msg11199 - (view) Author: Keyton Weissinger (keytonw) Date: 2002-06-15 03:31
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?
msg11200 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2002-06-18 04:19
Logged In: YES 
user_id=80475

Agree that it is a bug.
Working on a patch.
msg11201 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2002-06-19 18:13
Logged In: YES 
user_id=80475

Added patch.

Note, the patch would be much shorter if mangle() in 
compile.c were made non-static.
msg11202 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2002-06-19 19:30
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?
msg11203 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2002-06-20 02:51
Logged In: YES 
user_id=80475

Done.  New patch attached. Okay to commit?
msg11204 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2002-06-20 03:04
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).
msg11205 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2002-06-20 05:19
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.
msg11206 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2002-06-20 18:17
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).
msg11207 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2002-06-20 21:38
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).
msg11208 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2002-06-20 21:44
Logged In: YES 
user_id=6380

Hm, I don't like the code duplication. :-(

I liked the previous version better.
msg11209 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2002-06-20 22:25
Logged In: YES 
user_id=80475

Commited the previous version (with the Null check) as:
typeobject.c 2.156
compile.c 2.246
Python.h 2.52
test_descr.py 1.143

Closing bug.
History
Date User Action Args
2022-04-10 16:05:25adminsetgithub: 36751
2002-06-15 00:54:41keytonwcreate