classification
Title: Unify and optimize the helper for getting a builtin object
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.8, Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: kristjan.jonsson, pitrou, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2018-12-09 11:01 by serhiy.storchaka, last changed 2018-12-11 13:14 by serhiy.storchaka. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 11047 merged serhiy.storchaka, 2018-12-09 11:10
PR 11107 merged serhiy.storchaka, 2018-12-11 07:10
PR 11108 merged miss-islington, 2018-12-11 08:52
PR 11117 merged serhiy.storchaka, 2018-12-11 12:01
Messages (6)
msg331424 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-12-09 11:01
_PyIter_GetBuiltin() was introduced in issue14288 (31668b8f7a3efc7b17511bb08525b28e8ff5f23a). This was used for getting references to the builtin "iter" and "reverse". It was renamed to _PyObject_GetBuiltin() in a701388de1135241b5a8e4c970e06c0e83a66dc0.

There is other code that gets references to the builtin "getattr" using PyEval_GetBuiltins(). It is more efficient, but contains bugs.

The proposed PR unifies getting references to builtins:

* The prefix _PyObject_ is changed to _PyEval_, since this function has relation not to the object type but to the evaluation environment.

* It uses now the private _Py_Identifier API instead of a raw C string. This saves time by omitting the creation of a Unicode object on every call.

* It uses now fast PyEval_GetBuiltins() instead of slower PyImport_Import().

* Fixed errors handling in the code that used PyEval_GetBuiltins() before. It no longer swallows unexpected exceptions, no longer returns an error without setting an exception, and no longer causes raising a SystemError.

An example of an error in current code:

>>> import builtins
>>> del builtins.getattr
>>> int.bit_length.__reduce__()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
SystemError: NULL object passed to Py_BuildValue
msg331580 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-12-11 06:28
New changeset bb86bf4c4eaa30b1f5192dab9f389ce0bb61114d by Serhiy Storchaka in branch 'master':
bpo-35444: Unify and optimize the helper for getting a builtin object. (GH-11047)
https://github.com/python/cpython/commit/bb86bf4c4eaa30b1f5192dab9f389ce0bb61114d
msg331592 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-12-11 08:51
New changeset 3cae16d2e98ffaa89ddd311df70a857dfaff4020 by Serhiy Storchaka in branch '3.7':
bpo-35444: Fix error handling when fail to look up builtin "getattr". (GH-11047) (GH-11107)
https://github.com/python/cpython/commit/3cae16d2e98ffaa89ddd311df70a857dfaff4020
msg331598 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-12-11 10:13
New changeset be6ec444729f727f304ae10f3a7e2feda3cc3aaa by Serhiy Storchaka (Miss Islington (bot)) in branch '3.6':
bpo-35444: Fix error handling when fail to look up builtin "getattr". (GH-11047) (GH-11107) (GH-11108)
https://github.com/python/cpython/commit/be6ec444729f727f304ae10f3a7e2feda3cc3aaa
msg331608 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-12-11 11:03
I reopen the issuue. Commit bb86bf4c4eaa30b1f5192dab9f389ce0bb61114d introduced a new compiler warning:

gcc -pthread -c -Wno-unused-result -Wsign-compare -g -Og -Wall -O0   -std=c99 -Wextra -Wno-unused-result -Wno-unused-parameter -Wno-missing-field-initializers -Werror=implicit-function-declaration  -I./Include/internal  -I. -I./Include    -DPy_BUILD_CORE -o Objects/object.o Objects/object.c

In file included from ./Include/object.h:715,
                 from ./Include/pytime.h:6,
                 from ./Include/Python.h:75,
                 from Objects/object.c:4:
./Include/cpython/object.h:37:51: warning: 'PyId_builtins' defined but not used [-Wunused-variable]
 #define _Py_IDENTIFIER(varname) _Py_static_string(PyId_##varname, #varname)
                                                   ^~~~~
./Include/cpython/object.h:36:66: note: in definition of macro '_Py_static_string'
 #define _Py_static_string(varname, value)  static _Py_Identifier varname = _Py_static_string_init(value)
                                                                  ^~~~~~~
Objects/object.c:20:1: note: in expansion of macro '_Py_IDENTIFIER'
 _Py_IDENTIFIER(builtins);
 ^~~~~~~~~~~~~~
msg331619 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-12-11 13:14
New changeset 7211d306d4c2f73732540759e20dd17bd18b3361 by Serhiy Storchaka in branch 'master':
Remove an unused variable after bpo-35444. (GH-11117)
https://github.com/python/cpython/commit/7211d306d4c2f73732540759e20dd17bd18b3361
History
Date User Action Args
2018-12-11 13:14:34serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2018-12-11 13:14:21serhiy.storchakasetmessages: + msg331619
2018-12-11 12:01:32serhiy.storchakasetstage: resolved -> patch review
pull_requests: + pull_request10346
2018-12-11 11:03:30vstinnersetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg331608
2018-12-11 10:15:56serhiy.storchakasetstatus: open -> closed
stage: patch review -> resolved
resolution: fixed
versions: + Python 3.6, Python 3.7
2018-12-11 10:13:21serhiy.storchakasetmessages: + msg331598
2018-12-11 08:52:13miss-islingtonsetpull_requests: + pull_request10337
2018-12-11 08:51:32serhiy.storchakasetmessages: + msg331592
2018-12-11 07:10:51serhiy.storchakasetpull_requests: + pull_request10336
2018-12-11 06:28:21serhiy.storchakasetmessages: + msg331580
2018-12-09 11:10:59serhiy.storchakasetkeywords: + patch
stage: patch review
pull_requests: + pull_request10283
2018-12-09 11:01:30serhiy.storchakacreate