Unify and optimize the helper for getting a builtin object

[serhiy-storchaka]

BPO	35444
Nosy	@pitrou, @kristjanvalur, @vstinner, @serhiy-storchaka
PRs	bpo-35444: Unify and optimize the helper for getting a builtin object. #11047 [3.7] bpo-35444: Fix error handling when fail to look up builtin "getattr". (GH-11047) #11107 [3.6] bpo-35444: Fix error handling when fail to look up builtin "getattr". (GH-11047) (GH-11107) #11108 Fix the compiler warning after bpo-35444. #11117

^{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 2018-12-11.13:14:34.224>
created_at = <Date 2018-12-09.11:01:30.721>
labels = ['interpreter-core', '3.7', '3.8']
title = 'Unify and optimize the helper for getting a builtin object'
updated_at = <Date 2018-12-11.13:14:34.223>
user = 'https://github.com/serhiy-storchaka'

bugs.python.org fields:

activity = <Date 2018-12-11.13:14:34.223>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2018-12-11.13:14:34.224>
closer = 'serhiy.storchaka'
components = ['Interpreter Core']
creation = <Date 2018-12-09.11:01:30.721>
creator = 'serhiy.storchaka'
dependencies = []
files = []
hgrepos = []
issue_num = 35444
keywords = ['patch']
message_count = 6.0
messages = ['331424', '331580', '331592', '331598', '331608', '331619']
nosy_count = 4.0
nosy_names = ['pitrou', 'kristjan.jonsson', 'vstinner', 'serhiy.storchaka']
pr_nums = ['11047', '11107', '11108', '11117']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue35444'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

[serhiy-storchaka]

_PyIter_GetBuiltin() was introduced in bpo-14288 (31668b8). This was used for getting references to the builtin "iter" and "reverse". It was renamed to _PyObject_GetBuiltin() in a701388.

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

[serhiy-storchaka]

New changeset bb86bf4 by Serhiy Storchaka in branch 'master':
bpo-35444: Unify and optimize the helper for getting a builtin object. (GH-11047)
bb86bf4

[serhiy-storchaka]

New changeset 3cae16d by Serhiy Storchaka in branch '3.7':
bpo-35444: Fix error handling when fail to look up builtin "getattr". (GH-11047) (GH-11107)
3cae16d

[serhiy-storchaka]

New changeset be6ec44 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)
be6ec44

[vstinner]

I reopen the issuue. Commit bb86bf4 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);
 ^~~~~~~~~~~~~~

[serhiy-storchaka]

New changeset 7211d30 by Serhiy Storchaka in branch 'master':
Remove an unused variable after bpo-35444. (GH-11117)
7211d30