[Patches] [ python-Patches-659536 ] Use PyArg_UnpackTuple where possible

[noreply@sourceforge.net]

Patches item #659536, was opened at 2002-12-29 01:46
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=659536&group_id=5470

Category: None
Group: None
Status: Open
Resolution: Accepted
Priority: 5
Submitted By: Raymond Hettinger (rhettinger)
Assigned to: Raymond Hettinger (rhettinger)
Summary: Use PyArg_UnpackTuple where possible

Initial Comment:
Obtain cleaner coding and a system wide 
performance boost by using the fast, pre-parsed 
PyArg_Unpack function instead of PyArg_ParseTuple 
function which is driven by a format string.

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

>Comment By: Martin v. Löwis (loewis)
Date: 2002-12-29 16:11

Message:
Logged In: YES 
user_id=21627

The patch is already accepted, with the requested changes to
correctness (yours and mine). Your suggested changes to
replace other "OO" callers should be separately submitted
and reviewed (using METH_O where applicable also); your
suggested changes to use METH_NOARGS should be submitted as
yet another patch: reviewing this stuff is difficult, as it
is easy to overlook errors.

It is not the case that 
"" #OP
must be used, 
#OP
works just as fine. # is a unary operator, not a binary one
(## is the binary operator in the preprocessor). #OP creates
a string, so you have two string literals after one another
in the preprocessor output. This is not an error, as
subsequent string literals are concatenated in a later
translation phase in the compiler.

In the specific case, there is no point in pasting "" with
another string literal, so this should be dropped for clarity.


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

Comment By: Walter Dörwald (doerwalter)
Date: 2002-12-29 15:55

Message:
Logged In: YES 
user_id=89016

The patch looks good, apart from the following:the last
patched line in Python/bltinmodule.c has
   if (!PyArg_UnpackTuple(args, "OO:issubclass", 2, 2,
&derived, &cls))
instead of
   if (!PyArg_UnpackTuple(args, "issubclass", 2, 2,
&derived, &cls))

"" #OP must be used, because OP is not passed in as a string
constant, i.e. the macro is called as spami(truth,
PyObject_IsTrue), not as spami("truth", PyObject_IsTrue), so
IMHO that part of the patch is OK.

There are several more spots, when this could be used:
Modules/_codecsmodule.c: register
Modules/_hotshot.c: runcall
Modules/_localemodule.c: strcoll
Modules/_sre.c: __deepcopy__, expand, stat, end, span
Modules/_tkinter.c: deletefilehandler, _flatten
Modules/arraymodule.c: count, index, remove, extend, append,
 tofile, fromlist
Modules/cPickle.c: Unpickler, load
Modules/cStringIO.c: getval, writelines, StringIO
Modules/mpzmodule.c: several unnamed functions
Modules/puremodule.c: purify_map_pool_id
Modules/pyexpat.c: ParseFile
Modules/selectmodule.c: select, unregister, poll
Modules/timemodule.c asctime, mktime
Modules/_bsddb.c: append, has_key, keys, items, values
Modules/posixmodule.c: setgid
Modules/typeobject.c: Many unnamed functions
PC/_winreg.c: CloseKey,  FlushKey, QueryInfoKey
Python/exception.c: __getitem__ and __str__ three times
Python/marshal.c: dump, load, dumps
Python/sysmodule.c: exit

Furthermore, , there are several spots where the format
string starts with :, i.e. ":foo". Should those functions
all be changed to METH_NOARGS?

Martin, can this patch be accepted, so it can go in before
2.3a1 is released?


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

Comment By: Martin v. Löwis (loewis)
Date: 2002-12-29 09:08

Message:
Logged In: YES 
user_id=21627

One more change: Please try to report the correct function
name for min/max, using ?:.

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

Comment By: Martin v. Löwis (loewis)
Date: 2002-12-29 09:00

Message:
Logged In: YES 
user_id=21627

The changes to spam* are non-sensical: there is no point in
concatenating the empty string with #OP, just remove the
empty string.

Apart from this, the patch is fine.

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

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