Merge duplicated _Py_IDENTIFIER identifiers in C code

[vstinner]

BPO	19514
Nosy	@loewis, @vstinner, @sigmavirus24
Files	patch_19514.diff merge_py_identifiers.patch merge_py_identifiers_sorted.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2013-11-07.17:48:57.137>
created_at = <Date 2013-11-07.01:02:53.162>
labels = ['easy']
title = 'Merge duplicated _Py_IDENTIFIER identifiers in C code'
updated_at = <Date 2013-11-07.20:51:13.316>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2013-11-07.20:51:13.316>
actor = 'python-dev'
assignee = 'none'
closed = True
closed_date = <Date 2013-11-07.17:48:57.137>
closer = 'loewis'
components = []
creation = <Date 2013-11-07.01:02:53.162>
creator = 'vstinner'
dependencies = []
files = ['32531', '32532', '32533']
hgrepos = []
issue_num = 19514
keywords = ['patch', 'easy']
message_count = 13.0
messages = ['202300', '202302', '202308', '202310', '202347', '202349', '202350', '202352', '202356', '202358', '202375', '202376', '202380']
nosy_count = 7.0
nosy_names = ['loewis', 'vstinner', 'python-dev', 'icordasc', 'seydou', 'elixir', 'fhahn']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue19514'
versions = ['Python 3.4']

[vstinner]

Some C files use more than once the same _Py_IDENTIFIER identifier. It would be more efficient to merge duplicated identifiers. Just move the definition to the top of the file.

_Py_IDENTIFIER(as_integer_ratio): Modules/_datetimemodule.c:1569
_Py_IDENTIFIER(as_integer_ratio): Modules/_datetimemodule.c:1668

_Py_IDENTIFIER(cursor): Modules/_sqlite/connection.c:1282
_Py_IDENTIFIER(cursor): Modules/_sqlite/connection.c:1312
_Py_IDENTIFIER(cursor): Modules/_sqlite/connection.c:1342

_Py_IDENTIFIER(fromutc): Modules/_datetimemodule.c:4210
_Py_IDENTIFIER(fromutc): Modules/_datetimemodule.c:4249
_Py_IDENTIFIER(fromutc): Modules/_datetimemodule.c:4812

_Py_IDENTIFIER(len): Objects/typeobject.c:5071
_Py_IDENTIFIER(len): Objects/typeobject.c:5235

_Py_IDENTIFIER(insert): Modules/_bisectmodule.c:198
_Py_IDENTIFIER(insert): Modules/_bisectmodule.c:93

_Py_IDENTIFIER(isoformat): Modules/_datetimemodule.c:2638
_Py_IDENTIFIER(isoformat): Modules/_datetimemodule.c:3596
_Py_IDENTIFIER(isoformat): Modules/_datetimemodule.c:4532

_Py_IDENTIFIER(strftime): Modules/_datetimemodule.c:1280
_Py_IDENTIFIER(strftime): Modules/_datetimemodule.c:2679

[vstinner]

See also issue bpo-19515 which is more general.

[vstinner]

Oh, I forgot:

_Py_IDENTIFIER(delitem): Objects/typeobject.c:5132
_Py_IDENTIFIER(delitem): Objects/typeobject.c:5183

[elixir]

I'll provide a patch later tonight.

[fhahn]

I've merged the _Py_IDENTIFIER identifiers mentioned above.

I stumbled over anohter instance where _Py_IDENTIFIER is used more than once:

_Py_IDENTIFIER(setitem) : Objects/typeobject.c#l5133
_Py_IDENTIFIER(setitem) : Objects/typeobject.c#l5184

[loewis]

As a matter of style, I suggest that all identifiers are moved to the top of a file if some of them live there. IOW, it's (IMO) unstylish to have some at the top, and some in the middle (although this works perfectly fine, of course).

[loewis]

Another matter of style: I suggest alphabetical order for the identifiers, at least when the list gets long.

[elixir]

The patch I promised above.

[elixir]

I added a new patch with sorted _Py_IDENTIFIERs.

Regarding all identifiers at the top, I guess it might be more stylish, but it might affect performance. I'm not sure, though.

[vstinner]

merge_py_identifiers_sorted.patch looks good to me.

[python-dev]

New changeset 4a09cc62419b by Martin v. Löwis in branch 'default':
Issue bpo-19514: Deduplicate some _Py_IDENTIFIER declarations.
http://hg.python.org/cpython/rev/4a09cc62419b

[loewis]

Thanks for the patch.

Note: moving all identifiers would not have made a difference. They are static variables, so from a run-time point of view, there is no difference whether they are inside or outside of functions.

[python-dev]

New changeset cb4c964800af by Victor Stinner in branch 'default':
Issue bpo-19514: Add Andrei Dorian Duma to Misc/ACKS for changeset 4a09cc62419b
http://hg.python.org/cpython/rev/cb4c964800af