-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add __builtins__ to globals argument of py::exec
and py::eval
if not present
#2616
Conversation
Fixes #1654 as well. |
Thanks a lot @YannickJadoul |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me as-is, but what do you think about moving the new code into inline void detail::ensure_global_contains_builtins()
? That way you'd only need one #if
(inside that function).
I'm not entirely convinced on which side to pick here. On a high level, definitely, avoid code duplication. In practice ... I'm a bit in doubt whether it looks better. But I guess it also doesn't look worse. |
One or two reviews of this last commit, please? |
Thanks, all! |
Description
Python (even 2.7!) adds
__builtins__
when callingexec
oreval
:For the C API's
PyRun_String
, this only happens since Python 3.8 (python/cpython#13362; see #1091 (comment)). It would make sense to me to backport/downport/sideport/port this to pybind11 on all versions of Python, for the sake of consistency. No?Closes #1091
Closes #2557
Closes #1654 (thanks, @bstaletic!)
Suggested changelog entry: