[Patches] [ python-Patches-711448 ] Warn about inter-module assignments shadowing builtins

SourceForge.net noreply@sourceforge.net
Mon, 09 Jun 2003 12:16:37 -0700


Patches item #711448, was opened at 2003-03-28 17:12
Message generated for change (Comment added) made by nascheme
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=711448&group_id=5470

Category: Core (C code)
Group: None
>Status: Closed
>Resolution: Accepted
Priority: 5
Submitted By: Neil Schemenauer (nascheme)
Assigned to: Neil Schemenauer (nascheme)
Summary: Warn about inter-module assignments shadowing builtins

Initial Comment:
The attached patch modifies module tp_setattro to warn
about
code that adds a name to the globals of another module that
shadows a builtin.  Unfortunately, there are other ways to
modify module globals (e.g. using vars() and mutating the
dictionary).

There are a few issues with module objects that I'm not
clear
about.  For example, do modules always have a md_dict that
is a PyDictObject?

----------------------------------------------------------------------

>Comment By: Neil Schemenauer (nascheme)
Date: 2003-06-09 19:16

Message:
Logged In: YES 
user_id=35752

Commited as moduleobject.c 2.46 (with nit fixed).

----------------------------------------------------------------------

Comment By: Guido van Rossum (gvanrossum)
Date: 2003-06-09 16:05

Message:
Logged In: YES 
user_id=6380

Looks good to me.

One nit: PyEval_GetBuiltins() doesn't ever return NULL, but
it if were to return NULL, it wouldn't set an exception. In
that case the assumption would be made by the code that an
exception was set. Perhaps you can combine this with a
typecheck on the return, so that you write e.g.

if (builtins == NULL || !PyDict_Check(builtins)) {
  PyErr_SetString(PyExc_SystemError, "no builtins
directory?!?!");
  return NULL;
}


----------------------------------------------------------------------

Comment By: Neil Schemenauer (nascheme)
Date: 2003-06-07 23:06

Message:
Logged In: YES 
user_id=35752

Here's a new version that uses PyEval_GetBuiltins() to get
the builtin module dict and then builds a set of builtin
names from that.  Is using PyEval_GetBuiltins() safe?

----------------------------------------------------------------------

Comment By: Guido van Rossum (gvanrossum)
Date: 2003-04-20 21:20

Message:
Logged In: YES 
user_id=6380

The basic idea of the patch is sound.

Why can't you check whether the name is in
__builtin__.__dict__ ?

----------------------------------------------------------------------

Comment By: Neil Schemenauer (nascheme)
Date: 2003-04-20 16:17

Message:
Logged In: YES 
user_id=35752

Is it okay if I move the list to the bltinmodule as
a non-static variable?  That would make it easier
for the compiler to use the list if or when it was
changed to optimize builtin lookup.

Also, is the basic idea of this patch sound?

----------------------------------------------------------------------

Comment By: Guido van Rossum (gvanrossum)
Date: 2003-04-16 19:01

Message:
Logged In: YES 
user_id=6380

Hm, it's kind of ugly to list all built-in names explicitly;
this list will be out of sync whenever a new built-in is
added. Can't you access the __builtin__ module and use its
keys? (One way to get the __builtin__ module is to use
PySys_GetObject("modules"), which should give you
sys.modules, and take it from there; you can count on
__builtin__ to be there, and if it's not, forget the whole
thing.)

----------------------------------------------------------------------

Comment By: Neil Schemenauer (nascheme)
Date: 2003-03-28 17:15

Message:
Logged In: YES 
user_id=35752

Attaching patch.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=711448&group_id=5470