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: Use PyArg_UnpackTuple where possible
Type: Stage:
Components: None Versions:
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: doerwalter, loewis, rhettinger
Priority: normal Keywords: patch

Created on 2002-12-29 00:46 by rhettinger, last changed 2022-04-10 16:06 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
unpack.diff rhettinger, 2002-12-29 00:48
Messages (6)
msg42131 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2002-12-29 00:46
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.
msg42132 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2002-12-29 08:00
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.
msg42133 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2002-12-29 08:08
Logged In: YES 
user_id=21627

One more change: Please try to report the correct function
name for min/max, using ?:.
msg42134 - (view) Author: Walter Dörwald (doerwalter) * (Python committer) Date: 2002-12-29 14:55
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?
msg42135 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2002-12-29 15:11
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.
msg42136 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2002-12-29 16:35
Logged In: YES 
user_id=80475

Removed the "" blocks in operator.c.
Fixed the "OO:" prefix for issubclass().
Applied patch and closing.

Thanks for the quick and thorough
reviews.
History
Date User Action Args
2022-04-10 16:06:03adminsetgithub: 37657
2002-12-29 00:46:35rhettingercreate