[Python-Dev] Deprecation warning 'assignment shadows builtin' doesn't take __builtins__ into account
Troels Therkelsen
troels@2-10.org
Mon, 7 Jul 2003 16:38:52 +0200
This is a multi-part message in MIME format.
------=_NextPart_000_0000_01C344A6.403D6090
Content-Type: text/plain;
charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Hi python-dev,
I noticed this behaviour:
Python 2.3b2 (#1, Jun 30 2003, 13:04:39)
[GCC 2.95.3 20010315 (release)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys, new
>>> d = {'__builtins__':{}}
>>> d['mod'] = new.module('mod')
>>> d['mod'].__builtins__ = d['__builtins__']
>>> exec "mod.reload = 42" in d
<string>:1: DeprecationWarning: assignment shadows builtin
>>> d['sys'] = sys
>>> exec "print sys._getframe().f_builtins" in d
{}
Surely, the DeprecationWarning shouldn't be printed if you use
__builtins__ to explicitly define your own builtins dict? I mean,
the 'reload' name in this case is an unknown name until it is
defined in mod.
I looked through the archive and found the thread which describes
why this change was done, namely so that, eventually, the current
builtin name checking can be moved from runtime to compile time.
Shouldn't the compile time check be able to take __builtins__
into account? Assuming the answer to that is 'yes' then shouldn't
the current DeprecationWarning also be able to deal with it?
Looking into the code (Objects/moduleobject.c), I discovered that
the code does deal with it, but since it assigns to a static
variable, the builtin names are found once and then for every
subsequent check, the previously calculated dict is used.
However, doing this means that you get the errorenous (well, imho)
warning above. And since DeprecationWarnings usually indicate that
the operation will be illegal at some point, I wanted to ensure that
custom builtins via __builtins__ doesn't have this error.
Therefore, I submit for your approval/scorn/pointing and laughing
the attached patch. It deals with the problem in a way that involves
no change unless PyEval_GetBuiltins() returns a builtins dict
different from PyThreadState_Get()->interp->builtin (the builtins
dict should never change thus unless a custom __builtins__ is in
effect as far as I can tell). In the case that a different
builtins is detected, a local list of builtin names is used for
that call to module_setattr(). The way the problem is solved is
rather naive, I admit, but I didn't want to put any more work into
it before I get confirmation from someone that I'm on the right
track :-).
One thing I don't understand is why the current code goes to all
this trouble to create a new dict from the dict that
PyEval_GetBuiltins() returns, instead of just checking directly.
Ie., doing
if (PyDict_GetItem(PyEval_GetBuiltins(), name) != NULL) {
instead of
int shadows = shadows_builtin(globals, name);
if (shadows == 1) {
I know that the latter will emulate a non-changable builtins dict,
assuming that the first time this code is called,
current_frame->f_builtins == PyThreadState_Get()->interp->builtins.
As far as I can tell, this is a safe assumption.
I also noticed that there's no test in Lib/test/test_module.py
which verifies that shadowing builtin names raises a warning (and,
indeed, that not shadowing one does not raise a warning). Is
this intentional? I'm willing to write that simple test if should
be there.
Sorry for the long post, feel free to flog me with a wet fish ;-)
Regards,
Troels Therkelsen
------=_NextPart_000_0000_01C344A6.403D6090
Content-Type: application/octet-stream;
name="py2.diff"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment;
filename="py2.diff"
Index: Objects/moduleobject.c=0A=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A=
RCS file: =
/var/local/cvs/python-tarball/python/dist/src/Objects/moduleobject.c,v=0A=
retrieving revision 2.46=0A=
diff -c -r2.46 moduleobject.c=0A=
*** Objects/moduleobject.c 9 Jun 2003 18:41:54 -0000 2.46=0A=
--- Objects/moduleobject.c 6 Jul 2003 23:55:58 -0000=0A=
***************=0A=
*** 199,209 ****=0A=
}=0A=
=0A=
static PyObject *=0A=
! find_builtin_names(void)=0A=
{=0A=
! PyObject *builtins, *names, *key, *value;=0A=
int pos =3D 0;=0A=
- builtins =3D PyEval_GetBuiltins();=0A=
if (builtins =3D=3D NULL || !PyDict_Check(builtins)) {=0A=
PyErr_SetString(PyExc_SystemError, "no builtins dict!");=0A=
return NULL;=0A=
--- 199,208 ----=0A=
}=0A=
=0A=
static PyObject *=0A=
! find_builtin_names(PyObject *builtins)=0A=
{=0A=
! PyObject *names, *key, *value;=0A=
int pos =3D 0;=0A=
if (builtins =3D=3D NULL || !PyDict_Check(builtins)) {=0A=
PyErr_SetString(PyExc_SystemError, "no builtins dict!");=0A=
return NULL;=0A=
***************=0A=
*** 224,247 ****=0A=
return names;=0A=
}=0A=
=0A=
/* returns 0 or 1 (and -1 on error) */=0A=
static int=0A=
shadows_builtin(PyObject *globals, PyObject *name)=0A=
{=0A=
static PyObject *builtin_names =3D NULL;=0A=
! if (builtin_names =3D=3D NULL) {=0A=
! builtin_names =3D find_builtin_names();=0A=
! if (builtin_names =3D=3D NULL)=0A=
! return -1;=0A=
! }=0A=
! if (!PyString_Check(name))=0A=
return 0;=0A=
! if (PyDict_GetItem(globals, name) =3D=3D NULL &&=0A=
! PyDict_GetItem(builtin_names, name) !=3D NULL) {=0A=
! return 1;=0A=
}=0A=
else {=0A=
! return 0;=0A=
}=0A=
}=0A=
=0A=
--- 223,272 ----=0A=
return names;=0A=
}=0A=
=0A=
+ static int=0A=
+ builtin_has_name(PyObject *builtins, PyObject *name)=0A=
+ {=0A=
+ PyObject *custom_builtins =3D find_builtin_names(builtins);=0A=
+ int ret;=0A=
+ =0A=
+ if (custom_builtins =3D=3D NULL) {=0A=
+ return -1;=0A=
+ }=0A=
+ =0A=
+ ret =3D (PyDict_GetItem(custom_builtins, name) !=3D NULL);=0A=
+ Py_DECREF(custom_builtins);=0A=
+ return ret;=0A=
+ }=0A=
+ =0A=
/* returns 0 or 1 (and -1 on error) */=0A=
static int=0A=
shadows_builtin(PyObject *globals, PyObject *name)=0A=
{=0A=
static PyObject *builtin_names =3D NULL;=0A=
! PyObject *builtins =3D PyEval_GetBuiltins();=0A=
! =0A=
! if (!PyString_Check(name)=0A=
! || PyDict_GetItem(globals, name) !=3D NULL=0A=
! || builtins =3D=3D globals) {=0A=
return 0;=0A=
! }=0A=
! =0A=
! if (builtins =3D=3D PyThreadState_Get()->interp->builtins) {=0A=
! if (builtin_names =3D=3D NULL) {=0A=
! builtin_names =3D find_builtin_names(builtins);=0A=
! if (builtin_names =3D=3D NULL) {=0A=
! return -1;=0A=
! }=0A=
! }=0A=
! if (PyDict_GetItem(builtin_names, name) !=3D NULL) {=0A=
! return 1;=0A=
! }=0A=
! else {=0A=
! return 0;=0A=
! }=0A=
}=0A=
else {=0A=
! return builtin_has_name(builtins, name);=0A=
}=0A=
}=0A=
=0A=
***************=0A=
*** 249,256 ****=0A=
module_setattr(PyObject *m, PyObject *name, PyObject *value)=0A=
{=0A=
PyObject *globals =3D ((PyModuleObject *)m)->md_dict;=0A=
! PyObject *builtins =3D PyEval_GetBuiltins();=0A=
! if (globals !=3D NULL && globals !=3D builtins) {=0A=
int shadows =3D shadows_builtin(globals, name);=0A=
if (shadows =3D=3D 1) {=0A=
if (PyErr_Warn(PyExc_DeprecationWarning,=0A=
--- 274,280 ----=0A=
module_setattr(PyObject *m, PyObject *name, PyObject *value)=0A=
{=0A=
PyObject *globals =3D ((PyModuleObject *)m)->md_dict;=0A=
! if (globals !=3D NULL) {=0A=
int shadows =3D shadows_builtin(globals, name);=0A=
if (shadows =3D=3D 1) {=0A=
if (PyErr_Warn(PyExc_DeprecationWarning,=0A=
------=_NextPart_000_0000_01C344A6.403D6090--