[Python-Dev] Deprecation warning 'assignment shadows builtin' doesn't take builtins into account (original) (raw)

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 :1: DeprecationWarning: assignment shadows builtin d['sys'] = sys exec "print sys.getframe().fbuiltins" 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=

--- 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=

! 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--